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

[Peter Zijlstra]

On Wed, Sep 09, 2026 at 02:13:11PM +0200, Frederic Weisbecker wrote:

> > > I argue that's not possible:
> > > 
> > > A:   sigqueue stores
> > > 
> > > B:   AQUIRE tasklist
> > > 
> > > C:   exit_state store
> > > 
> > > D:   RELEASE tasklist
> > > 
> > > E                               ACQUIRE tasklist

> > > F                               if (exit_state)
> > > 				    	swap_pid()
> > > G                                   STORE_PID
> > > 
> > > 				      RELEASE tasklist
> > > 
> > > H                                                       READ PID
> > > 							      ....
> > > I                                		              ACQUIRE siglock

> > 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.
> 
> I didn't know about all those UNLOCK+LOCK properties. Well,
> I know that UNLOCK+LOCK on the same lock, or on different locks
> but the same CPU, equals smp_mb() except on powerpc. Which is why
> we have smp_mb__after_unlock_lock(). But what you describe is quite
> different.
> 
> Is this something that we should expect litmus to modelize?

IIRC these commits:

6e89e831a901 ("tools/memory-model: Add extra ordering for locks and remove it for ordinary release/acquire")
ddfe12944e84 ("tools/memory-model: Provide extra ordering for unlock+lock pair on the same CPU")

Were supposed to handle:

	CPU0		CPU1

	UNLOCK(A)
			LOCK(A)

and

	CPU0

	UNLOCK(A)
	LOCK(B)

respectively. I'm forever confused by the actual CAT stuff, nor am I
particularly adept at these litmus things. Boqun, Alan?

> Because the following doesn't verify that:
> ---
> C MP+farfetched
> 
> {}
> 
> P0(int *next, int *prev, int *exit_state, spinlock_t *tasklist_lock)
> {
> 	// list_del_init()
> 	WRITE_ONCE(*next, 1);
> 	WRITE_ONCE(*prev, 1);
> 	// exit_notify()
> 	spin_lock(tasklist_lock);
> 	WRITE_ONCE(*exit_state, 1);
> 	spin_unlock(tasklist_lock);
> }
> 
> P1(int *exit_state, int *pid, spinlock_t *tasklist_lock)
> {
> 	int r0;
> 
> 	// de_thread()
> 	spin_lock(tasklist_lock);
> 	r0 = READ_ONCE(*exit_state);
> 	if (r0 == 1) {
> 		// exchange_tids()
> 		WRITE_ONCE(*pid, 1);
> 	}
> 	spin_unlock(tasklist_lock);
> }
> 
> P2(int *next, int *prev, int *pid, spinlock_t *sighand)
> {
> 	int r0;
> 	int r1;
> 	// get target
> 	r0 = READ_ONCE(*pid);
> 	spin_lock(sighand);
> 	// queue signal
> 	r1 = READ_ONCE(*next);
> 	if (r1 == 0)
> 		WRITE_ONCE(*prev, 2);
> 	spin_unlock(sighand);
> }
> 
> exists (prev=1 /\ 2:r0=1) (* Bad outcome. *)
> ---
> herd7 -conf linux-kernel.cfg ~/farfetched.litmus 
> Test MP+farfetched Allowed
> States 4
> 2:r0=0; [prev]=1;
> 2:r0=0; [prev]=2;
> 2:r0=1; [prev]=1;
> 2:r0=1; [prev]=2;
> Ok
> Witnesses
> Positive: 2 Negative: 7
> Condition exists ([prev]=1 /\ 2:r0=1)
> Observation MP+farfetched Sometimes 2 7
> Time MP+farfetched 0.02
> Hash=a44733c870613a81ae096a93babe215