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

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Sep 09, 2026 at 11:08:31AM +0200, Thomas Gleixner wrote:
> On Wed, Sep 09 2026 at 10:04, Peter Zijlstra wrote:
> > On Tue, Sep 08, 2026 at 12:15:21PM +0200, Frederic Weisbecker wrote:
> > 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.
> 
> That's irrelevant because in the above scenario posix_timer_fn() 't'
> points to the exiting old leader (on the left) because the PID store has
> not happened yet and it therefore observes PF_EXITING on it so it won't
> touch the sigqueue. Note, that setting and checking PF_EXITING is
> serialized by sighand lock, so this is fine.

There is nothing that constraints the 3rd column from happening before,
it could happen after transfer_pid().

> > The easy solution would probably be to do transfer_pid() while holding
> > siglock?
> 
> That'd be only relevant for the situation Frederic is concerned about,
> i.e. the case where the third party observes the TID swap.

That is the case I was aiming at.

> Because with that visible 't' in posix_timer_send_sigqueue() won't be
> old_leader, which has PF_EXITING set, it will be new_leader which has it
> not set.

Same as above, there is nothing constraining the 3rd column from sliding
up or down. If it manages to see the new_leader, I don't see why it
would see the sigqueue flush.

> So Frederic is concerned that posix_timer_send_sigqueue() can observe
> the PID store but not observe the sigqueue stores.
> 
> I argue that's not possible:
> 
> A:  sigqueue stores
> 
> B:   AQUIRE tasklist
> 
> C:   exit_state store
> 
> D:   RELEASE tasklist
>      // sigqueue and exit_state stores become globally visible
>  ------------------------------------------------------------------------
> 
> E                               ACQUIRE tasklist
>  ------------------------------------------------------------------------
> F                                if (exit_state)
> 				   swap_pid()
> G                                     STORE_PID
> 
> 				// The PID store can become visible in the
> 				// system right here so F can observe them before
> 				// RELEASE tasklist

The STORE_PID is not a STORE_RELEASE.

> H                                                             READ PID

And this READ is not LOAD_AQUIRE; although the LOCK siglock is probably
sufficient here. The READ MUST happen before LOCK siglock by means of
data dependency, and then the LOCK will constrain later loads.

> 							      ....
> I                                		              ACQUIRE siglock
> 
> After #A the sigqueue stores are maybe visible
> 
> After #C the exit_state store is maybe visible
> 
> After #D both #A and #C are guaranteed to be visible to _ALL_ agents in
> the system and cannot become magically become invisible after that
> point.

No, that is not in fact how Power (or ARM) works AFAICT. Memory ordering
is not global. It is entirely possible some CPUs see a store while
others do not.

The only guarantee here is that IF you acquire tasklist_lock (you
observe the store that unlocked it), you will also observe preceding
stores.  But since the posix_timer_fn() column does not in fact observe
or care about tasklist_lock, there is no ordering.

> The new leader cannot swap PIDs before acquiring task list lock and
> before it observed exit_state != 0 under it. That's fully serialized
> against the old leader as both hold task list lock for their operations.
> 
> #F creates a control dependency, so if the new leader acquires task list
>    lock before the old it will observe 0, drop the lock and wait. No PID
>    store obviously.

A control dependency only ensure *that* CPU will complete the exit_state
load before the store, it is a local LOAD->STORE ordering.

> #G can be come visible immediately but is only guaranteed to be visible
>    globally at the RELEASE of tasklist lock.

Nope, not at all. Can be randomly visible to random sets of CPUs.

> #H can only observe the PID store after the store actually happened in
>    #G. So it either reads the original PID or the swapped PID.

Sure. But that has no bearing on if it sees the sigqueue stores at A.

> #I is not really relevant for this. It's only relevant for PF_EXITING
>    and other stuff which is directly protected by it. And it does not
>    matter whether it locks the old or the new sighand.
> 
> Now let's look at the full chain and what can possibly be visible or not
> and when:
> 
> #A can trickle into the tasklist held section, but not after #D.

Yup.

> #C cannot be reordered against #B and #D

Agreed.

> #A is therefore guaranteed to be globally visible _before_ new leader
>    observes exit_state != 0 in #F under task list lock

Nope, A is therefore visible if you acquire tasklist_lock, specifically,
when you observe the store from D. And only if that matching LOAD is a
LOAD-ACQUIRE, such that subsequent loads are forced to be later.

> #G cannot be reordered against #F and obviously not against #E either.

Indeed.

>    It can become visible at any point after the store, but as argued
>    above that visibility can't be reordered before #A (sigqueue stores)
>    became visible.

Let G' be the unnamed RELEASE after G.

Now, I have deleted and rewritten this tail end at least twice now. And
I *think* I'm agreeing with you. Let me explain:

It all hinges on D-E and H-I.

D-E is a UNLOCK+LOCK hand-over, which is not quite the same as
RELEASE+ACQUIRE. Specifically, we have:

  RELEASE+ACQUIRE: RCpc,  only the CPUs involved agree on the ordering
  UNLOCK+LOCK:     RCtso, the hand-over is store-ordering

So while earlier I was arguing with RCpc in mind, in which case D-E
completely goes away and we can consider B-G' to be one big critical
section from the PoV of a third CPU (our posix_timer_fn() one). In this
case we can push A down and G up and have them cross.

*However*, since these are locks, we actually have D-E be UNLOCK+LOCK,
which is RCtso and that *does* impose store order, so A stores must
happen before G stores

Combine with H-I, which has a data dependency from the LOAD to the LOCK
and thereby constraints later LOADs, those sigqueue loads that come
after I must in fact observe the A stores.