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. -- To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
