Re: [patch V2 1/8] signal: Prevent exec() race

[Frederic Weisbecker]

Le Wed, Sep 09, 2026 at 02:03:42AM +0200, Oleg Nesterov a écrit :
> On 09/08, Frederic Weisbecker wrote:
> >
> > Old leader                           Exec'ing New leader     CPU 2
> > -----                                -----                   -----
> >
> > WRITE q->next = q
> > WRITE q->prev = q
> 
> Damn ;) I am shy to suggest this again, but if we have _any_
> concerns about the races with list_del_init()...
> 
> The lockless flush_sigqueue_list() doesn't need to it, right?
> So perhaps something like below (on top of this series) to avoid
> the (potential) race explicitly?
> 
> Most probably this change is wrong. And in any case we can do better.
> Just to explain what I mean.
> 
> In short, I mean that exit_signals() -> flush path can do
> __sigqueue_free(q) without list_del_init(&q->list).
> 
> Yes! I agree in advance that if this change can fix something,
> then it fixes the symptom.
> 
> But at the same time, why does flush_sigqueue_list(head) need
> list_del_init() before __sigqueue_free() ? AFAICS only to make
> list_empty(head) == true. This looks confusing to me.
> 
> Oh, I am sure I missed something again ;)
> 
> Oleg.
> ---
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index f93d8f77ec1a..32c344ee4769 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -457,14 +457,19 @@ static void __sigqueue_free(struct sigqueue *q)
>  	kmem_cache_free(sigqueue_cachep, q);
>  }
>  
> -static void flush_sigqueue_list(struct list_head *head)
> +static void __flush_sigqueue_list(struct list_head *head)
>  {
>  	struct sigqueue *q, *tmp;
>  
> -	list_for_each_entry_safe(q, tmp, head, list) {
> -		list_del_init(&q->list);
> +	list_for_each_entry_safe(q, tmp, head, list)
>  		__sigqueue_free(q);
> -	}
> +}

I must confess that leaves me an uncomfortable taste :-)

What do you think about Peter's solution?

Thanks.

-- 
Frederic Weisbecker
SUSE Labs