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

[Heiko Carstens]

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 ;)

> diff --cc arch/s390/kernel/ptrace.c
> index cea17010448f,ac1dc74632b0..000000000000
> --- a/arch/s390/kernel/ptrace.c
> +++ b/arch/s390/kernel/ptrace.c
> @@@ -821,6 -821,16 +821,8 @@@ long compat_arch_ptrace(struct task_str
>   
>   asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
>   {
>  -	long ret = 0;
> + 	unsigned long mask = -1UL;
> + 
>  -	/* Do the secure computing check first. */
>  -	if (secure_computing()) {
>  -		/* seccomp failures shouldn't expose any additional code. */
>  -		ret = -1;
>  -		goto out;
>  -	}
>  -
>   	/*
>   	 * The sysc_tracesys code in entry.S stored the system
>   	 * call number to gprs[2].
> @@@ -846,11 -850,15 +848,14 @@@
>   	if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
>   		trace_sys_enter(regs, regs->gprs[2]);
>   
> - 	audit_syscall_entry(regs->gprs[2], regs->orig_gpr2,
> - 			    regs->gprs[3], regs->gprs[4],
> - 			    regs->gprs[5]);
> - 
> + #ifdef CONFIG_COMPAT
> +         if (test_thread_flag(TIF_31BIT))
> +                 mask = 0xffffffff;
> + #endif

Better: use is_compat_task() and avoid yet another ifdef.

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