Re: [patch 3/3] futex: Prevent attaching to kernel threads

From: Thomas Gleixner
Date: Mon May 12 2014 - 18:18:04 EST


On Mon, 12 May 2014, Davidlohr Bueso wrote:
> On Mon, 2014-05-12 at 20:45 +0000, Thomas Gleixner wrote:
> > We happily allow userspace to declare a random kernel thread to be the
> > owner of a user space PI futex.
> >
> > Found while analysing the fallout of Dave Jones syscall fuzzer.
> >
> > Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > ---
> > kernel/futex.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > Index: linux-2.6/kernel/futex.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/futex.c
> > +++ linux-2.6/kernel/futex.c
> > @@ -682,6 +682,8 @@ static struct task_struct * futex_find_g
> > p = find_task_by_vpid(pid);
> > if (p)
> > get_task_struct(p);
> > + else
> > + p = NULL;

Duh, that hunk is actually left over from the attempt to do what you
suggest below :(

> >
> > rcu_read_unlock();
> >
> > @@ -814,6 +816,11 @@ lookup_pi_state(u32 uval, struct futex_h
> > if (!p)
> > return -ESRCH;
> >
> > + if (!p->mm) {
> > + put_task_struct(p);
> > + return -EPERM;
> > + }
>
> It might make sense to check the mm inside futex_find_get_task(), and
> simply never take the refcount if NULL.

Right, but for some reason it was a more ugly patch than the one I
posted (w/o the stupid hunk above) and I tried to minimize the impact
for stable backporting.

Thanks,

tglx




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/