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

[Oleg Nesterov <oleg@tv-sign.ru>]

Andrew Morton wrote:
> 
> Oleg Nesterov <oleg@tv-sign.ru> wrote:
> >
> > 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.

unshare_sighand() is ok, it never populates *new_sighp, it just returns
errror code: 0 when ->sighand is not shared, EINVAL otherwise.

I argued about 'if (new_sigh)' code in sys_unshare() because it lies about
locking rules.

Btw, copy_process() forbids CLONE_SIGHAND without CLONE_VM (is there a
good reason for that?), but one can do unshare(CLONE_VM). This is odd.

Oleg.