lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 23 Jun 2016 08:01:13 +0200
From:	Heiko Carstens <heiko.carstens@...ibm.com>
To:	Stephen Rothwell <sfr@...b.auug.org.au>
Cc:	Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
	linux-next@...r.kernel.org, linux-kernel@...r.kernel.org,
	Kees Cook <keescook@...omium.org>,
	Martin Schwidefsky <schwidefsky@...ibm.com>
Subject: Re: linux-next: manual merge of the audit tree with the security tree

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?

Powered by blists - more mailing lists