Re: [PATCH] fix broken class-based syscall audit

On Mon, 2007-05-14 at 10:56 -0500, Klaus Weidner wrote: > On Mon, May 14, 2007 at 05:51:50PM +0200, Marcus Meissner wrote: > > On Mon, May 14, 2007 at 10:46:36AM -0500, Klaus Weidner wrote: > > > The sanity check in audit_match_class() is wrong, AUDIT_BITMASK_SIZE is > > > 64, providing space for 2048 syscalls in 64 * 32bit integers. The > > > comparison only supports 256 syscalls (sizeof __u32 is 4), and silently > > > returns "no match" for valid higher-numbered syscalls. > [...] > > > --- linux-2.6.18.i686/kernel/auditfilter.c.lspp.80 2007-05-11 17:06:08.000000000 -0500 > > > +++ linux-2.6.18.i686/kernel/auditfilter.c 2007-05-11 17:09:37.000000000 -0500 > > > @@ -306,7 +306,7 @@ > > > > > > int audit_match_class(int class, unsigned syscall) > > > { > > > - if (unlikely(syscall >= AUDIT_BITMASK_SIZE * sizeof(__u32))) > > > + if (unlikely(syscall >= AUDIT_BITMASK_SIZE * 32)) > > > return 0; > > > if (unlikely(class >= AUDIT_SYSCALL_CLASSES || !classes[class])) > > > return 0; > > > > You likely need to fix audit_register_class() if this is true. > > I don't see a problem in audit_register_class() - it correctly uses > sizeof(__u32) for allocating the memory since that's counted in bytes, > only the comparison needs to count bits. If that's the problem wouldn't we be better to use sizeof(__u32) *8 rather than hard code the 32 in there? (sidebar: is it portable to assume sizeof() returns bytes and *8 is the right way to go on all archs?) That would make it easier to do u32 search/replace in the future if we ever have to grow this stuff....