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

[Frederic Weisbecker <frederic@kernel.org>]

Le Wed, Sep 09, 2026 at 10:04:07AM +0200, Peter Zijlstra a écrit :
> On Tue, Sep 08, 2026 at 12:15:21PM +0200, Frederic Weisbecker wrote:
> 
> > Yes this side is well ordered but what about the other side.
> > Ok let's simplify the picture:
> > 
> > Old leader                           Exec'ing New leader     CPU 2
> > -----                                -----                   -----
> > 
> > WRITE q->next = q
> > WRITE q->prev = q
> > 
> > ACQUIRE tasklist
> > RELEASE tasklist
> >                                      ACQUIRE tasklist
> >                                      RELEASE tasklist
> > 
> >                                      WRITE pid
> >                                                              READ pid
> > 							     // smp_mb()
> >                                                              if q->next == q
> >                                                                  WRITE q->prev
> > 
> > Isn't there a missing pairing full barrier in CPU 2 ?
> 
> 
> Let me try and have a go :-)
> 
> 
> do_exit()				de_thread() 			posix_timer_fn()
>   exit_signal()				  LOCK siglock 			  posix_timer_send_sigqueue()
>     LOCK siglock			  UNLOCK siglock 		    t = posix_timer_get_target()
>     tsk->flags |= PF_EXITING; 						    LOCK siglock
>     UNLOCK siglock			  if (!thread_group_leader) 	      if (!list_empty(sigqueue))
>     					    LOCK tasklist_lock
>     flush_sigqueue_list();		    if (leader->exit_state)
> 					      break;
>   ...                                       transfer_pid()
> 					    UNLOCK tasklist_lock
>   exit_notify()
>     LOCK tasklist_lock
>     tsk->exit_state = EXIT_ZOMBIE;
>     UNLOCK tasklist_lock
> 
> 
> 
> Then there is indeed nothing that makes sure posix_timer_fn() sees
> sigqueue updates done by do_exit(), because those are ordered by
> tasklist_lock, but posix_timer_fn() doesn't care about that.
> 
> The easy solution would probably be to do transfer_pid() while holding
> siglock?

That should work, especially with a big fat comment, and the tasklist_lock ->
sighand lock dependency already exists.

Thanks!

-- 
Frederic Weisbecker
SUSE Labs