Re: [PATCH] exit: fix lost-update race on the child_subreaper bitfields

[Oleg Nesterov <oleg@redhat.com>]

On 10/09, Nickolai Zeldovich wrote:
>
> is_child_subreaper, has_child_subreaper and autoreap are 1-bit fields
> that share one word of struct signal_struct.  A store to any of them is
> a read-modify-write of the whole word, and two of the stores run
> without a common lock:

Yes, thanks, this is obviously wrong. But I am a bit confused...

> autoreap is only written to
> a not yet published signal_struct in copy_process() and is not racy by
> itself,

Yes,

> but it shares the word and is converted with the other two.

So why? If you turn the *_child_subreaper members to bool, then
autoreap:1 is fine?

Oleg.

> --- a/include/linux/sched/signal.h
> +++ b/include/linux/sched/signal.h
> @@ -130,9 +130,9 @@ struct signal_struct {
>  	 * process will inherit a flag if they should look for a
>  	 * child_subreaper process at exit.
>  	 */
> -	unsigned int		is_child_subreaper:1;
> -	unsigned int		has_child_subreaper:1;
> -	unsigned int		autoreap:1;
> +	bool			is_child_subreaper;
> +	bool			has_child_subreaper;
> +	bool			autoreap;