On Fri, Jun 24, 2016 at 11:05:33AM -0400, Paul Moore wrote:

quoted

quoted

quoted

quoted

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