On Sun, Sep 18, 2016 at 07:28:46PM +0100, Ben Hutchings wrote:
> On Sun, Sep 18, 2016 at 05:05:12PM +0200, Jann Horn wrote:
> > This prevents an attacker from determining the robust_list or
> > compat_robust_list userspace pointer of a process created by executing
> > a setuid binary. Such an attack could be performed by racing
> > get_robust_list() with a setuid execution. The impact of this issue is that
> > an attacker could theoretically bypass ASLR when attacking setuid binaries.
> > 
> > Signed-off-by: Jann Horn <jann@thejh.net>
> > ---
> >  kernel/futex.c        | 31 +++++++++++++++++++++----------
> >  kernel/futex_compat.c | 31 +++++++++++++++++++++----------
> >  2 files changed, 42 insertions(+), 20 deletions(-)
> > 
> > diff --git a/kernel/futex.c b/kernel/futex.c
> > index 46cb3a3..002f056 100644
> > --- a/kernel/futex.c
> > +++ b/kernel/futex.c
> > @@ -3007,31 +3007,42 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
> >  	if (!futex_cmpxchg_enabled)
> >  		return -ENOSYS;
> >  
> > -	rcu_read_lock();
> > -
> > -	ret = -ESRCH;
> > -	if (!pid)
> > +	if (!pid) {
> >  		p = current;
> > -	else {
> > +		get_task_struct(p);
> > +	} else {
> > +		rcu_read_lock();
> >  		p = find_task_by_vpid(pid);
> > -		if (!p)
> > -			goto err_unlock;
> > +		/* pin the task to permit dropping the RCU read lock before
> > +		 * acquiring the mutex
> > +		 */
> > +		get_task_struct(p);
> 
> get_task_struct() requires a non-null pointer so you can't move the
> null check below it.

Oh, right, thanks. Will fix that in v2.