On Mon, May 14, 2007 at 04:47:29PM -0400, Eric Paris wrote:
> 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....
The AUDIT_WORD and AUDIT_BIT macros in include/linux/audit.h assume that
they'll fit 32 bits into each __u32:
#define AUDIT_WORD(nr) ((__u32)((nr)/32))
#define AUDIT_BIT(nr) (1 << ((nr) - AUDIT_WORD(nr)*32))
(If the actual capacity of the __u32 were larger they would continue
using only 32 bits.)
I think the hardcoded 32 is appropriate. I'm not sure about the exact
sizeof() standard semantics, but I remember that the standard doesn't
assume 8-bit bytes. However, I think it's safe to assume that a __u32
can fit 32 bits, if not a lot of other code would break also.
-Klaus
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit
