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@xxxxxxxxx>
> > ---
> > 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.
Attachment:
signature.asc
Description: Digital signature
