On Wednesday, May 28, 2014 10:43:57 PM Eric Paris wrote:
> On Wed, 2014-05-28 at 19:27 -0700, Andy Lutomirski wrote:
> > On Wed, May 28, 2014 at 7:23 PM, Eric Paris <[email protected]> wrote:
> > > On Wed, 2014-05-28 at 18:44 -0700, Andy Lutomirski wrote:
> > >> Fixes an easy DoS and possible information disclosure.
> > >> 
> > >> This does nothing about the broken state of x32 auditing.
> > >> 
> > >> Cc: [email protected]
> > >> Signed-off-by: Andy Lutomirski <[email protected]>
> > >> ---
> > >>
> > >>  kernel/auditsc.c | 27 ++++++++++++++++++---------
> > >>  1 file changed, 18 insertions(+), 9 deletions(-)
> > >>
> > >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > >> index f251a5e..7ccd9db 100644
> > >> --- a/kernel/auditsc.c
> > >> +++ b/kernel/auditsc.c
> > >> @@ -728,6 +728,22 @@ static enum audit_state audit_filter_task(struct
> > >> task_struct *tsk, char **key)> >>
> > >>       return AUDIT_BUILD_CONTEXT;
> > >>  }
> > >>
> > >> +static bool audit_in_mask(const struct audit_krule *rule, unsigned
> > >> long val) +{
> > >> +     int word, bit;
> > >> +
> > >> +     if (val > 0xffffffff)
> > >> +             return false;
> > > 
> > > Why is this necessary?
> >
> > To avoid an integer overflow.  Admittedly, this particular overflow
> > won't cause a crash, but it will cause incorrect results.

So, what is the effect of this patch? Does it hide the syscall from the audit 
system? Does it fail the syscall?


> You know this code pre-dates git?  I admit, I'm shocked no one ever
> noticed it before!  This is ANCIENT.  And clearly broken.
> 
> I'll likely ask Richard to add a WARN_ONCE() in both this place, and
> below in word > AUDIT_BITMASK_SIZE so we might know if we ever need a
> larger bitmask to store syscall numbers....

We need absolute guarantees. Either its auditable or prevented - always.

-Steve

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to