Re: [PATCH] unshare: Use rcu_assign_pointer when setting sighand

[Andrew Morton]

Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> "Eric W. Biederman" wrote:
> > 
> > @@ -1573,7 +1573,7 @@ asmlinkage long sys_unshare(unsigned lon
> > 
> >                 if (new_sigh) {
> >                         sigh = current->sighand;
> > -                       current->sighand = new_sigh;
> > +                       rcu_assign_pointer(current->sighand, new_sigh);
> >                         new_sigh = sigh;
> >                 }
> 
> Isn't it better to just replace this code with
> 'BUG_ON(new_sigh != NULL)' ?
> 
> It is never executed, but totally broken, afaics.
> task_lock() has nothing to do with ->sighand changing.
> 

/*
 * Unsharing of sighand for tasks created with CLONE_SIGHAND is not
 * supported yet
 */
static int unshare_sighand(unsigned long unshare_flags, struct sighand_struct **new_sighp)

It's all just a place-holder at present.

If we don't plan on ever supporting unshare(CLONE_SIGHAND) we should take
that code out and make it return EINVAL.  Right now.

And because we don't presently support CLONE_SIGHAND we should return
EINVAL if it's set.  Right now.

And we should change sys_unshare() to reject not-understood flags.  Right
now.

If we don't do these things we'll silently break 2.6.16-back-compatibility
of applications which are coded for future kernels.

-
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/