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

[Heiko Carstens]

On Thu, Jun 23, 2016 at 12:14:11PM -0400, Paul Moore wrote:
> On Thu, Jun 23, 2016 at 2:01 AM, Heiko Carstens
> <heiko.carstens@xxxxxxxxxx> wrote:
> > On Thu, Jun 23, 2016 at 02:18:14PM +1000, Stephen Rothwell wrote:
> >> Hi Paul,
> >>
> >> Today's linux-next merge of the audit tree got a conflict in:
> >>
> >>   arch/s390/kernel/ptrace.c
> >>
> >> between commit:
> >>
> >>   0208b9445bc0 ("s390/ptrace: run seccomp after ptrace")
> >>
> >> from the security tree and commit:
> >>
> >>   bba696c2c083 ("s390: ensure that syscall arguments are properly masked on s390")
> >>
> >> from the audit tree.
> >
> > Hmm, I haven't seen that commit, therefore I'm just commenting on the
> > result ;)
> 
> It was sent to the linux-audit and linux-s390 mailing lists yesterday
> with a follow up comment that I was going to add it to the audit#next
> branch and if anyone had any objections to let me know.
> 
> * https://www.redhat.com/archives/linux-audit/2016-June/msg00051.html

Yes, I missed that, sorry!

> >> +     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.