Re: [PATCH] signal: Use list_del_init_careful() in flush_sigqueue()

[Frederic Weisbecker <frederic@kernel.org>]

Le Mon, Aug 24, 2026 at 01:54:26PM +0200, Oleg Nesterov a écrit :
> On 08/24, Oleg Nesterov wrote:
> >
> > On 08/24, Thomas Gleixner wrote:
> > >
> > > --- a/fs/exec.c
> > > +++ b/fs/exec.c
> > > @@ -983,6 +983,18 @@ static int de_thread(struct task_struct
> > >  		}
> > >
> > >  		/*
> > > +		 * Ensure that POSIX timer SIGEV_THREAD_ID signals pending for
> > > +		 * the former leader are removed under sighand::siglock _before_
> > > +		 * taking over the leader's TID. Otherwise the lockless cleanup
> > > +		 * in release_task() can race against a concurrent signal
> > > +		 * delivery to the new leader. The former leader has PF_EXITING
> > > +		 * set which prevents queueing of SIGEV_THREAD_ID signals up to
> > > +		 * the point where it's sighand gets cleared.
> > > +		 */
> > > +		scoped_guard(spinlock_irq, lock)
> > > +			flush_sigqueue(&leader->pending);
> 
> scoped_guard(spinlock_irq) is not right. This needs scoped_guard(spinlock),
> the code runs with irqs disabled.
> 
> > Hmm, at first glance... If we change de_thread() to do this _after_ transfer_pid's
> > (before release_task(leader)), then posixtimer_send_sigqueue() doesn't need any
> > changes, no?
> 
> IOW. Unless I am totally confused, we only need to flush the
> SIGQUEUE_PREALLOC sigqueue's which were sent to the (old) leader
> before it changed its pid. So we can do this
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index a14f28b15607..550367e7fe6c 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1029,6 +1029,9 @@ static int de_thread(struct task_struct *tsk)
>  		write_unlock_irq(&tasklist_lock);
>  		cgroup_threadgroup_change_end(tsk);
>  
> +		scoped_guard(spinlock_irq, lock)
> +			flush_sigqueue(&leader->pending);
> +

Is there something to prevent the timer from firing on another CPU,
racing with this tiny window and queue the signal to the old leader? After
all exchange_tids() is just some RCU pointers changed but there is nothing
to synchronize the readers before the flush_sigqueue(). So pid_task() may
still return the old leader after it?


>  		release_task(leader);

Thanks.

-- 
Frederic Weisbecker
SUSE Labs