Re: linux-next: manual merge of the audit tree with the security tree

[Heiko Carstens]

On Fri, Jun 24, 2016 at 11:05:33AM -0400, Paul Moore wrote:
> >> >> +     audit_syscall_entry(regs->gprs[2], regs->orig_gpr2 & mask,
> >> >> +                         regs->gprs[3] & mask, regs->gprs[4] & mask,
> >> >> +                         regs->gprs[5] & mask);
> >> >
> >> > With these masks it is more correct, however these are still not the values
> >> > used by the system call itself. This would be still incorrect for
> >> > e.g. compat pointers (31 bit on s390).
> >> >
> >> > So it seems like audit_syscall_entry should be called after all sign, zero
> >> > and masking has been done?
> >>
> >> For someone not familiar with s390, compat or not, where would you
> >> suggest we place the audit_syscall_entry() call?
> >
> > I was thinking of a more generic solution for all architectures: for
> > example setting a new TIF flag within do_syscall_trace_enter which
> > indicates that audit_syscall_entry needs be called and then add a
> > conditional call to the SYSCALL_DEFINE and COMPAT_SYSCALL_DEFINE macros.
> >
> > That way audit_syscall_entry would always receive already properly sign and
> > zero extended system call parameters. At the downside this would increase
> > the kernel text size by probably ~370 conditional branches and add two more
> > instructions on the system call hot path.
> >
> > But that's something that could be done independently from your patch,
> > which already improves the current situation.
> 
> My immediate concern is making sure that we are at least recording the
> arguments correctly in the audit record.  My simple tests look okay,
> but as I said before, I'm far from a s390 expert and your initial
> comment made it sound like there were still problems with how we were
> recording the arguments.  Can you either confirm that we are logging
> the arguments correctly, or provide a suggestion on how to get the
> right values?  That would be most helpful at this point.

The arguments are correct, except that they are missing sign and zero
extension to full 64 bit. However I would expect that the audit subsystem
will only work on the lower 32 bits anyway for compat tasks. So that
shouldn't be a problem.

I'm a bit concerned about user space pointers passed as argument for compat
tasks. These need to mask out 33 instead of 32 bits.  This is of course
system call specific and I don't know enough about audit to tell if it
could be a problem.