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

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

[Thomas Gleixner]

Hyunwoo debugged the following KASAN UAF splat:

  BUG: KASAN: slab-use-after-free in __send_signal_locked+0xb27/0xba0
  Write of size 8 at addr ffff888007ed80c8 by task poc/79
  ...
  Call Trace:
   __send_signal_locked+0xb27/0xba0
   do_send_sig_info+0xa7/0x160
   do_send_specific+0x76/0xa0
   __x64_sys_tgkill+0x193/0x270
  ...
  Allocated by task 80:
   do_timer_create+0x1a4/0x1030
   __x64_sys_timer_create+0x145/0x190
  ...
  Freed by task 12:
   kmem_cache_free_bulk+0x1f8/0x4a0
   kvfree_rcu_bulk+0x14f/0x1c0
   kfree_rcu_work+0x128/0x1a0
  ...
  Last potentially related work creation:
   kvfree_call_rcu+0x39/0x390
   __flush_itimer_signals+0x211/0x320
   flush_itimer_signals+0x47/0x90
   begin_new_exec+0xa6b/0x28c0

It turned out that this happens with a non-leader exec() as Hyunwoo
explained:

de_thread() calls exchange_tids() before release_task(leader), so the
struct pid held by a SIGEV_THREAD_ID timer created against the leader's tid
now points to the thread which called execve(). pid_task() returns that
thread and lock_task_sighand() on it succeeds.

If the timer signal is blocked, its sigqueue stays queued on the leader's
task::pending. The next expiry of that timer can then run while
release_task() flushes the queue.

posixtimer_send_sigqueue() checks whether the sigqueue is already queued
with a plain list_empty(), which only reads list_head::next.
list_del_init() is not atomic and INIT_LIST_HEAD() stores list_head::next
before list_head::prev, so the check can pass in between. list_add_tail()
queues the entry on the task::pending of the live thread, and the
list_head::prev store from the flush then overwrites the list_head::prev
link that list_add_tail() has just set.

__flush_itimer_signals() does not undo that either. With list_head::prev
pointing at the entry itself, its list_del_init() only stores the same
values again, so the entry is not removed from the list. It is still there
after the last reference is dropped and the timer is freed by RCU, and the
list_add_tail() of a later tgkill() follows that list_head::prev into the
freed timer.

This problem surfaced with the recent commit which moved the sigqueue flush
out of the sighand lock held region.

Hyonwoo proposed to fix this by using list_del_init_careful(), but that
just papers over the problem. After some disucssions and various attempts
to solve it, Eric pointed out that there is no reason to flush
task::pending late in release_task() and it should be done in
exit_signals() already.

As nothing can collect and deliver signals which are queued in a dying
task's pending queue, there is no reason to delay it further.

But it has to be ensured that no signals can be queued into it after that
point. exit_signals() sets PF_EXITING in task::flags, which can be used as
an indicator for this.

Cure it by:

  - Preventing signal queueing for task private signals (PIDTYPE_PID) when
    the task has PF_EXITING set in __send_signal_locked() and in
    posixtimer_send_sigqueue().

  - Protecting the unlocked setting of PF_EXITING in exit_signals() for the
    task group empty and the group exit case with sighand lock

  - Flushing task::pending signals right there.

    Optimize that by moving the whole pending list to an on-stack list head
    under sighand lock and free the signals without the lock held.

Fixes: fb3bbcfe344e ("exit: change the release_task() paths to call flush_sigqueue() lockless")
Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Debugged-by: Hyunwoo Kim <imv4bel@gmail.com>
Suggested-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Cc: stable@vger.kernel.org
Closes: https://patch.msgid.link/aok1rdkBgZsynHZB@v4bel
---
V4: Prevent requeuing of signals which are unignored - Oleg, Eric

    Split out the decision into an inline which can be reused by the posix
    CPU timer follow up changes.

V3: Restructure code and fix the missing unlock - Oleg

V2: Don't flush w/o sighand lock held - Oleg
    Move the while pending list under the lock and free it lockless
---
 kernel/exit.c   |   11 +++--
 kernel/signal.c |  103 +++++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 78 insertions(+), 36 deletions(-)

--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -299,12 +299,13 @@ void release_task(struct task_struct *p)
 	free_pids(post.pids);
 	release_thread(p);
 	/*
-	 * This task was already removed from the process/thread/pid lists
-	 * and lock_task_sighand(p) can't succeed. Nobody else can touch
-	 * ->pending or, if group dead, signal->shared_pending. We can call
-	 * flush_sigqueue() lockless.
+	 * This task was already removed from the process/thread/pid lists and
+	 * lock_task_sighand(p) can't succeed. If it's the group leader then
+	 * flush tsk->signal->shared_pending. tsk->pending has been flushed
+	 * already in exit_signals(). Nothing else can touch
+	 * signal->shared_pending anymore, so flush_sigqueue() can be invoked
+	 * lockless.
 	 */
-	flush_sigqueue(&p->pending);
 	if (thread_group_leader(p))
 		flush_sigqueue(&p->signal->shared_pending);
 
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -457,18 +457,28 @@ static void __sigqueue_free(struct sigqu
 	kmem_cache_free(sigqueue_cachep, q);
 }
 
-void flush_sigqueue(struct sigpending *queue)
+static void flush_sigqueue_list(struct list_head *head)
 {
-	struct sigqueue *q;
+	struct sigqueue *q, *tmp;
 
-	sigemptyset(&queue->signal);
-	while (!list_empty(&queue->list)) {
-		q = list_entry(queue->list.next, struct sigqueue , list);
+	list_for_each_entry_safe(q, tmp, head, list) {
 		list_del_init(&q->list);
 		__sigqueue_free(q);
 	}
 }
 
+void flush_sigqueue(struct sigpending *queue)
+{
+	sigemptyset(&queue->signal);
+	flush_sigqueue_list(&queue->list);
+}
+
+static void sigqueue_dequeue_pending(struct sigpending *queue, struct list_head *head)
+{
+	sigemptyset(&queue->signal);
+	list_splice_init(&queue->list, head);
+}
+
 /*
  * Flush all pending signals for this kthread.
  */
@@ -1019,6 +1029,21 @@ static inline bool legacy_queue(struct s
 	return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
 }
 
+/*
+ * When PF_EXITING is set the task is on the way out and has t::pending
+ * flushed already. Prevent queueing of PIDTYPE_PID signals as they would
+ * be leaked.
+ */
+static inline bool task_can_queue_signal(struct task_struct *t, enum pid_type type)
+{
+	lockdep_assert_held(&t->sighand->siglock);
+
+	if (!(t->flags & PF_EXITING))
+		return true;
+
+	return type != PIDTYPE_PID;
+}
+
 static int __send_signal_locked(int sig, struct kernel_siginfo *info,
 				struct task_struct *t, enum pid_type type, bool force)
 {
@@ -1030,6 +1055,10 @@ static int __send_signal_locked(int sig,
 	lockdep_assert_held(&t->sighand->siglock);
 
 	result = TRACE_SIGNAL_IGNORED;
+
+	if (!task_can_queue_signal(t, type))
+		goto ret;
+
 	if (!prepare_signal(sig, t, force))
 		goto ret;
 
@@ -1968,11 +1997,25 @@ static inline struct task_struct *posixt
 	struct task_struct *t = pid_task(tmr->it_pid, tmr->it_pid_type);
 
 	if (t && tmr->it_pid_type != PIDTYPE_PID &&
-	    same_thread_group(t, current) && !current->exit_state)
+	    same_thread_group(t, current) && !(current->flags & PF_EXITING))
 		t = current;
 	return t;
 }
 
+/*
+ * Find the target task for the POSIX timer signal and prevent that a
+ * PIDTYPE_PID signal is queued on a task which has PF_EXITING set.
+ */
+static inline struct task_struct *posixtimer_get_unignore_target(struct k_itimer *tmr)
+{
+	struct task_struct *t = posixtimer_get_target(tmr);
+
+	if (t && task_can_queue_signal(t, tmr->it_pid_type))
+		return t;
+
+	return NULL;
+}
+
 void posixtimer_send_sigqueue(struct k_itimer *tmr)
 {
 	struct sigqueue *q = &tmr->sigq;
@@ -1990,6 +2033,9 @@ void posixtimer_send_sigqueue(struct k_i
 	if (!likely(lock_task_sighand(t, &flags)))
 		return;
 
+	if (!task_can_queue_signal(t, tmr->it_pid_type))
+		goto unlock;
+
 	/*
 	 * Update @tmr::sigqueue_seq for posix timer signals with sighand
 	 * locked to prevent a race against dequeue_signal().
@@ -2081,6 +2127,7 @@ void posixtimer_send_sigqueue(struct k_i
 	result = TRACE_SIGNAL_DELIVERED;
 out:
 	trace_signal_generate(sig, &q->info, t, tmr->it_pid_type != PIDTYPE_PID, result);
+unlock:
 	unlock_task_sighand(t, &flags);
 }
 
@@ -2136,7 +2183,7 @@ static void posixtimer_sig_unignore(stru
 		 * has exited by now, drop the reference count.
 		 */
 		guard(rcu)();
-		target = posixtimer_get_target(tmr);
+		target = posixtimer_get_unignore_target(tmr);
 		if (target)
 			posixtimer_queue_sigqueue(&tmr->sigq, target, tmr->it_pid_type);
 		else
@@ -3120,42 +3167,36 @@ static void retarget_shared_pending(stru
 
 void exit_signals(struct task_struct *tsk)
 {
+	LIST_HEAD(sigq_list);
 	int group_stop = 0;
-	sigset_t unblocked;
 
 	/*
 	 * @tsk is about to have PF_EXITING set - lock out users which
-	 * expect stable threadgroup.
+	 * expect a stable threadgroup.
 	 */
 	cgroup_threadgroup_change_begin(tsk);
 
-	if (thread_group_empty(tsk) || (tsk->signal->flags & SIGNAL_GROUP_EXIT)) {
+	scoped_guard(spinlock_irq, &tsk->sighand->siglock) {
 		tsk->flags |= PF_EXITING;
-		cgroup_threadgroup_change_end(tsk);
-		return;
-	}
 
-	spin_lock_irq(&tsk->sighand->siglock);
-	/*
-	 * From now this task is not visible for group-wide signals,
-	 * see wants_signal(), do_signal_stop().
-	 */
-	tsk->flags |= PF_EXITING;
+		sigqueue_dequeue_pending(&tsk->pending, &sigq_list);
 
-	cgroup_threadgroup_change_end(tsk);
+		if (task_sigpending(tsk) && !thread_group_empty(tsk) &&
+		    !(tsk->signal->flags & SIGNAL_GROUP_EXIT)) {
+			sigset_t unblocked = tsk->blocked;
+
+			signotset(&unblocked);
+			retarget_shared_pending(tsk, &unblocked);
+
+			if (unlikely(tsk->jobctl & JOBCTL_STOP_PENDING) &&
+			    task_participate_group_stop(tsk))
+				group_stop = CLD_STOPPED;
+		}
+	}
 
-	if (!task_sigpending(tsk))
-		goto out;
+	cgroup_threadgroup_change_end(tsk);
 
-	unblocked = tsk->blocked;
-	signotset(&unblocked);
-	retarget_shared_pending(tsk, &unblocked);
-
-	if (unlikely(tsk->jobctl & JOBCTL_STOP_PENDING) &&
-	    task_participate_group_stop(tsk))
-		group_stop = CLD_STOPPED;
-out:
-	spin_unlock_irq(&tsk->sighand->siglock);
+	flush_sigqueue_list(&sigq_list);
 
 	/*
 	 * If group stop has completed, deliver the notification.  This

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

[Oleg Nesterov]

On 09/05, Thomas Gleixner wrote:
>
> +/*
> + * Find the target task for the POSIX timer signal and prevent that a
> + * PIDTYPE_PID signal is queued on a task which has PF_EXITING set.
> + */
> +static inline struct task_struct *posixtimer_get_unignore_target(struct k_itimer *tmr)
> +{
> +	struct task_struct *t = posixtimer_get_target(tmr);
> +
> +	if (t && task_can_queue_signal(t, tmr->it_pid_type))
> +		return t;
> +
> +	return NULL;
> +}
...
> @@ -2136,7 +2183,7 @@ static void posixtimer_sig_unignore(stru
>  		 * has exited by now, drop the reference count.
>  		 */
>  		guard(rcu)();
> -		target = posixtimer_get_target(tmr);
> +		target = posixtimer_get_unignore_target(tmr);
>  		if (target)
>  			posixtimer_queue_sigqueue(&tmr->sigq, target, tmr->it_pid_type);

Not sure posixtimer_get_unignore_target() makes a lot of sence...
It has a single caller, and posixtimer_sig_unignore() can do

		target = posixtimer_get_target(tmr);
		if (target && task_can_queue_signal(target, tmr->it_pid_type))
			posixtimer_queue_sigqueue(&tmr->sigq, target, tmr->it_pid_type);

But this is cosmetic and subjective. I believe the patch is correct.

Reviewed-by: Oleg Nesterov <oleg@redhat.com>

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

[Eric W. Biederman]

Thomas Gleixner <tglx@kernel.org> writes:

> @@ -1019,6 +1029,21 @@ static inline bool legacy_queue(struct s
>  	return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
>  }
>  
> +/*
> + * When PF_EXITING is set the task is on the way out and has t::pending
> + * flushed already. Prevent queueing of PIDTYPE_PID signals as they would
> + * be leaked.
> + */
> +static inline bool task_can_queue_signal(struct task_struct *t, enum pid_type type)
> +{
> +	lockdep_assert_held(&t->sighand->siglock);
> +
> +	if (!(t->flags & PF_EXITING))
> +		return true;
> +

I don't know if we care but I just noticed that this disallows
using tkill(..., SIGKILL) or tgkill(..., SIGKILL) to stop coredumps.

> +	return type != PIDTYPE_PID;
> +}
> +
>  static int __send_signal_locked(int sig, struct kernel_siginfo *info,
>  				struct task_struct *t, enum pid_type type, bool force)
>  {

Eric

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

[Oleg Nesterov]

On 09/06, Eric W. Biederman wrote:
>
> Thomas Gleixner <tglx@kernel.org> writes:
>
> > @@ -1019,6 +1029,21 @@ static inline bool legacy_queue(struct s
> >  	return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
> >  }
> >
> > +/*
> > + * When PF_EXITING is set the task is on the way out and has t::pending
> > + * flushed already. Prevent queueing of PIDTYPE_PID signals as they would
> > + * be leaked.
> > + */
> > +static inline bool task_can_queue_signal(struct task_struct *t, enum pid_type type)
> > +{
> > +	lockdep_assert_held(&t->sighand->siglock);
> > +
> > +	if (!(t->flags & PF_EXITING))
> > +		return true;
> > +
>
> I don't know if we care but I just noticed that this disallows
> using tkill(..., SIGKILL) or tgkill(..., SIGKILL) to stop coredumps.

I think we do care, but the coredumping thread is not PF_EXITING ?

Oleg.

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

[Thomas Gleixner]

On Sun, Sep 06 2026 at 17:39, Eric W. Biederman wrote:
> Thomas Gleixner <tglx@kernel.org> writes:
>> @@ -1019,6 +1029,21 @@ static inline bool legacy_queue(struct s
>>  	return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
>>  }
>>  
>> +/*
>> + * When PF_EXITING is set the task is on the way out and has t::pending
>> + * flushed already. Prevent queueing of PIDTYPE_PID signals as they would
>> + * be leaked.
>> + */
>> +static inline bool task_can_queue_signal(struct task_struct *t, enum pid_type type)
>> +{
>> +	lockdep_assert_held(&t->sighand->siglock);
>> +
>> +	if (!(t->flags & PF_EXITING))
>> +		return true;
>> +
>
> I don't know if we care but I just noticed that this disallows
> using tkill(..., SIGKILL) or tgkill(..., SIGKILL) to stop coredumps.

The thread running the coredump does not have PF_EXITING set:

get_signal()
        ....
        vfs_coredump()
        ...
        do_group_exit()

The only interaction with coredumps of a task which reached do_exit() is
via:

        synchronize_group_exit()
           coredump_task_exit()
        ...
        exit_signals() ; // sets PF_EXITING.

coredump_task_exit() waits until the dumper thread finished, so even if
tkill() is directed at a non-dumper thread which is stuck there in
coredump_task_exit() the signal will be queued and complete_signal()
will set signal->flags = SIGNAL_GROUP_EXIT and wake everyone up
including the dumper thread.

So the only case where this matters is when a task sets PF_EXITING
_before_ the dumper starts:

T1			T2
                        do_exit()
vfs_coredump()
                          synchronize_group_exit()
                            lock(sighand)
                            tsk->flags |= PF_POSTCOREDUMP;
                            core_state = signal->core_state;
                            unlock(sighand);
                            // core_state == NULL

		          exit_signals() // Sets PF_EXITING
  zap_threads()
    lock(sighand)
    // Observes T2->flags PF_POSTCOREDUMP
    // and skips T2

Now in current mainline a tkill(T2, SIGKILL) will queue the SIGKILL in
T2->pending, but complete_signal() will not turn it into a group exit
either because it is a PIDTYPE_PID signal when PF_EXITING is set:

complete_signal()
  // wants_signal() returns false because PF_EXITING is set
  if (wants_signal(sig, p)) 
      t = p;
  else if ((type == PIDTYPE_PID) || thread_group_empty(p))
      return;  // path taken because type == PIDTYPE_PID

So it is queued for nothing and just sitting in T2->pending until
flush_sigqueue() mops it up.

That has been so since:

  5fcd835bf8c2 ("signals: use __group_complete_signal() for the specific signals too")

which was merged 18 years ago in the 2.6.26 merge window.

Which means not queueing it in the first place has exactly the same
outcome vs. SIGKILL.

The only difference is that current mainline still reaches
signalfd_notify() further down in __send_signal_locked(), while with the
early exit it will not. Does it actually matter?

If it matters we could simply force PIDTYPE_TGID for SIGKILL if type ==
PIDTYPE_PID because complete_signal() converts SIGKILL into a group exit
anyway.

Thanks,

        tglx

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

[Frederic Weisbecker]

Le Sat, Sep 05, 2026 at 08:59:01PM +0200, Thomas Gleixner a écrit :
> Hyunwoo debugged the following KASAN UAF splat:
> 
>   BUG: KASAN: slab-use-after-free in __send_signal_locked+0xb27/0xba0
>   Write of size 8 at addr ffff888007ed80c8 by task poc/79
>   ...
>   Call Trace:
>    __send_signal_locked+0xb27/0xba0
>    do_send_sig_info+0xa7/0x160
>    do_send_specific+0x76/0xa0
>    __x64_sys_tgkill+0x193/0x270
>   ...
>   Allocated by task 80:
>    do_timer_create+0x1a4/0x1030
>    __x64_sys_timer_create+0x145/0x190
>   ...
>   Freed by task 12:
>    kmem_cache_free_bulk+0x1f8/0x4a0
>    kvfree_rcu_bulk+0x14f/0x1c0
>    kfree_rcu_work+0x128/0x1a0
>   ...
>   Last potentially related work creation:
>    kvfree_call_rcu+0x39/0x390
>    __flush_itimer_signals+0x211/0x320
>    flush_itimer_signals+0x47/0x90
>    begin_new_exec+0xa6b/0x28c0
> 
> It turned out that this happens with a non-leader exec() as Hyunwoo
> explained:
> 
> de_thread() calls exchange_tids() before release_task(leader), so the
> struct pid held by a SIGEV_THREAD_ID timer created against the leader's tid
> now points to the thread which called execve(). pid_task() returns that
> thread and lock_task_sighand() on it succeeds.
> 
> If the timer signal is blocked, its sigqueue stays queued on the leader's
> task::pending. The next expiry of that timer can then run while
> release_task() flushes the queue.
> 
> posixtimer_send_sigqueue() checks whether the sigqueue is already queued
> with a plain list_empty(), which only reads list_head::next.
> list_del_init() is not atomic and INIT_LIST_HEAD() stores list_head::next
> before list_head::prev, so the check can pass in between. list_add_tail()
> queues the entry on the task::pending of the live thread, and the
> list_head::prev store from the flush then overwrites the list_head::prev
> link that list_add_tail() has just set.
> 
> __flush_itimer_signals() does not undo that either. With list_head::prev
> pointing at the entry itself, its list_del_init() only stores the same
> values again, so the entry is not removed from the list. It is still there
> after the last reference is dropped and the timer is freed by RCU, and the
> list_add_tail() of a later tgkill() follows that list_head::prev into the
> freed timer.
> 
> This problem surfaced with the recent commit which moved the sigqueue flush
> out of the sighand lock held region.
> 
> Hyonwoo proposed to fix this by using list_del_init_careful(), but that
> just papers over the problem. After some disucssions and various attempts
> to solve it, Eric pointed out that there is no reason to flush
> task::pending late in release_task() and it should be done in
> exit_signals() already.
> 
> As nothing can collect and deliver signals which are queued in a dying
> task's pending queue, there is no reason to delay it further.
> 
> But it has to be ensured that no signals can be queued into it after that
> point. exit_signals() sets PF_EXITING in task::flags, which can be used as
> an indicator for this.
> 
> Cure it by:
> 
>   - Preventing signal queueing for task private signals (PIDTYPE_PID) when
>     the task has PF_EXITING set in __send_signal_locked() and in
>     posixtimer_send_sigqueue().
> 
>   - Protecting the unlocked setting of PF_EXITING in exit_signals() for the
>     task group empty and the group exit case with sighand lock
> 
>   - Flushing task::pending signals right there.
> 
>     Optimize that by moving the whole pending list to an on-stack list head
>     under sighand lock and free the signals without the lock held.
> 
> Fixes: fb3bbcfe344e ("exit: change the release_task() paths to call flush_sigqueue() lockless")
> Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
> Debugged-by: Hyunwoo Kim <imv4bel@gmail.com>
> Suggested-by: "Eric W. Biederman" <ebiederm@xmission.com>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Cc: stable@vger.kernel.org
> Closes: https://patch.msgid.link/aok1rdkBgZsynHZB@v4bel
> ---
> V4: Prevent requeuing of signals which are unignored - Oleg, Eric
> 
>     Split out the decision into an inline which can be reused by the posix
>     CPU timer follow up changes.
> 
> V3: Restructure code and fix the missing unlock - Oleg
> 
> V2: Don't flush w/o sighand lock held - Oleg
>     Move the while pending list under the lock and free it lockless
> ---
>  kernel/exit.c   |   11 +++--
>  kernel/signal.c |  103 +++++++++++++++++++++++++++++++++++++++-----------------
>  2 files changed, 78 insertions(+), 36 deletions(-)
> 
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -299,12 +299,13 @@ void release_task(struct task_struct *p)
>  	free_pids(post.pids);
>  	release_thread(p);
>  	/*
> -	 * This task was already removed from the process/thread/pid lists
> -	 * and lock_task_sighand(p) can't succeed. Nobody else can touch
> -	 * ->pending or, if group dead, signal->shared_pending. We can call
> -	 * flush_sigqueue() lockless.
> +	 * This task was already removed from the process/thread/pid lists and
> +	 * lock_task_sighand(p) can't succeed. If it's the group leader then
> +	 * flush tsk->signal->shared_pending. tsk->pending has been flushed
> +	 * already in exit_signals(). Nothing else can touch
> +	 * signal->shared_pending anymore, so flush_sigqueue() can be invoked
> +	 * lockless.
>  	 */
> -	flush_sigqueue(&p->pending);
>  	if (thread_group_leader(p))
>  		flush_sigqueue(&p->signal->shared_pending);
>  
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -457,18 +457,28 @@ static void __sigqueue_free(struct sigqu
>  	kmem_cache_free(sigqueue_cachep, q);
>  }
>  
> -void flush_sigqueue(struct sigpending *queue)
> +static void flush_sigqueue_list(struct list_head *head)
>  {
> -	struct sigqueue *q;
> +	struct sigqueue *q, *tmp;
>  
> -	sigemptyset(&queue->signal);
> -	while (!list_empty(&queue->list)) {
> -		q = list_entry(queue->list.next, struct sigqueue , list);
> +	list_for_each_entry_safe(q, tmp, head, list) {
>  		list_del_init(&q->list);
>  		__sigqueue_free(q);
>  	}
>  }
>  
> +void flush_sigqueue(struct sigpending *queue)
> +{
> +	sigemptyset(&queue->signal);
> +	flush_sigqueue_list(&queue->list);
> +}
> +
> +static void sigqueue_dequeue_pending(struct sigpending *queue, struct list_head *head)
> +{
> +	sigemptyset(&queue->signal);
> +	list_splice_init(&queue->list, head);
> +}
> +
>  /*
>   * Flush all pending signals for this kthread.
>   */
> @@ -1019,6 +1029,21 @@ static inline bool legacy_queue(struct s
>  	return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
>  }
>  
> +/*
> + * When PF_EXITING is set the task is on the way out and has t::pending
> + * flushed already. Prevent queueing of PIDTYPE_PID signals as they would
> + * be leaked.
> + */
> +static inline bool task_can_queue_signal(struct task_struct *t, enum pid_type type)
> +{
> +	lockdep_assert_held(&t->sighand->siglock);
> +
> +	if (!(t->flags & PF_EXITING))
> +		return true;
> +
> +	return type != PIDTYPE_PID;
> +}
> +
>  static int __send_signal_locked(int sig, struct kernel_siginfo *info,
>  				struct task_struct *t, enum pid_type type, bool force)
>  {
> @@ -1030,6 +1055,10 @@ static int __send_signal_locked(int sig,
>  	lockdep_assert_held(&t->sighand->siglock);
>  
>  	result = TRACE_SIGNAL_IGNORED;
> +
> +	if (!task_can_queue_signal(t, type))
> +		goto ret;
> +
>  	if (!prepare_signal(sig, t, force))
>  		goto ret;
>  
> @@ -1968,11 +1997,25 @@ static inline struct task_struct *posixt
>  	struct task_struct *t = pid_task(tmr->it_pid, tmr->it_pid_type);
>  
>  	if (t && tmr->it_pid_type != PIDTYPE_PID &&
> -	    same_thread_group(t, current) && !current->exit_state)
> +	    same_thread_group(t, current) && !(current->flags & PF_EXITING))
>  		t = current;
>  	return t;
>  }
>  
> +/*
> + * Find the target task for the POSIX timer signal and prevent that a
> + * PIDTYPE_PID signal is queued on a task which has PF_EXITING set.
> + */
> +static inline struct task_struct *posixtimer_get_unignore_target(struct k_itimer *tmr)
> +{
> +	struct task_struct *t = posixtimer_get_target(tmr);
> +
> +	if (t && task_can_queue_signal(t, tmr->it_pid_type))
> +		return t;
> +
> +	return NULL;
> +}
> +
>  void posixtimer_send_sigqueue(struct k_itimer *tmr)
>  {
>  	struct sigqueue *q = &tmr->sigq;
> @@ -1990,6 +2033,9 @@ void posixtimer_send_sigqueue(struct k_i
>  	if (!likely(lock_task_sighand(t, &flags)))
>  		return;
>  
> +	if (!task_can_queue_signal(t, tmr->it_pid_type))
> +		goto unlock;
> +
>  	/*
>  	 * Update @tmr::sigqueue_seq for posix timer signals with sighand
>  	 * locked to prevent a race against dequeue_signal().
> @@ -2081,6 +2127,7 @@ void posixtimer_send_sigqueue(struct k_i
>  	result = TRACE_SIGNAL_DELIVERED;
>  out:
>  	trace_signal_generate(sig, &q->info, t, tmr->it_pid_type != PIDTYPE_PID, result);
> +unlock:
>  	unlock_task_sighand(t, &flags);
>  }
>  
> @@ -2136,7 +2183,7 @@ static void posixtimer_sig_unignore(stru
>  		 * has exited by now, drop the reference count.
>  		 */
>  		guard(rcu)();
> -		target = posixtimer_get_target(tmr);
> +		target = posixtimer_get_unignore_target(tmr);
>  		if (target)
>  			posixtimer_queue_sigqueue(&tmr->sigq, target, tmr->it_pid_type);
>  		else
> @@ -3120,42 +3167,36 @@ static void retarget_shared_pending(stru
>  
>  void exit_signals(struct task_struct *tsk)
>  {
> +	LIST_HEAD(sigq_list);
>  	int group_stop = 0;
> -	sigset_t unblocked;
>  
>  	/*
>  	 * @tsk is about to have PF_EXITING set - lock out users which
> -	 * expect stable threadgroup.
> +	 * expect a stable threadgroup.
>  	 */
>  	cgroup_threadgroup_change_begin(tsk);
>  
> -	if (thread_group_empty(tsk) || (tsk->signal->flags & SIGNAL_GROUP_EXIT)) {
> +	scoped_guard(spinlock_irq, &tsk->sighand->siglock) {
>  		tsk->flags |= PF_EXITING;
> -		cgroup_threadgroup_change_end(tsk);
> -		return;
> -	}
>  
> -	spin_lock_irq(&tsk->sighand->siglock);
> -	/*
> -	 * From now this task is not visible for group-wide signals,
> -	 * see wants_signal(), do_signal_stop().
> -	 */
> -	tsk->flags |= PF_EXITING;
> +		sigqueue_dequeue_pending(&tsk->pending, &sigq_list);
>  
> -	cgroup_threadgroup_change_end(tsk);
> +		if (task_sigpending(tsk) && !thread_group_empty(tsk) &&
> +		    !(tsk->signal->flags & SIGNAL_GROUP_EXIT)) {
> +			sigset_t unblocked = tsk->blocked;
> +
> +			signotset(&unblocked);
> +			retarget_shared_pending(tsk, &unblocked);
> +
> +			if (unlikely(tsk->jobctl & JOBCTL_STOP_PENDING) &&
> +			    task_participate_group_stop(tsk))
> +				group_stop = CLD_STOPPED;
> +		}
> +	}
>  
> -	if (!task_sigpending(tsk))
> -		goto out;
> +	cgroup_threadgroup_change_end(tsk);
>  
> -	unblocked = tsk->blocked;
> -	signotset(&unblocked);
> -	retarget_shared_pending(tsk, &unblocked);
> -
> -	if (unlikely(tsk->jobctl & JOBCTL_STOP_PENDING) &&
> -	    task_participate_group_stop(tsk))
> -		group_stop = CLD_STOPPED;
> -out:
> -	spin_unlock_irq(&tsk->sighand->siglock);
> +	flush_sigqueue_list(&sigq_list);

It probably doesn't matter in practice, I don't know feel free to ignore,
but FWIW it looks like it's still vulnerable to the theoretical far fetched
race I described. The head is moved under the lock but individual nodes are
deleted without the lock.

CPU 0                                CPU 1                   CPU 2
-----                                -----                   -----

exit_signals()
   spin_lock(sighand)
   tsk->flags |= PF_EXITING;
   list_splice_init(&queue->list, head);
   spin_unlock(sighand)

   list_for_each_safe(head, node)
      list_del_init(node)
         node->next = node // A
         node->prev = node // B
...
                                     de_thread()
                                        // acquired tsk->flags
                                        // and signal flushed
                                        // through tasklist_lock
                                        transfer_pid() // C
                                                           
                                                           posix_timer_fn()
                                                              posixtimer_send_sigqueue()
                                                                 // OBSERVES C
                                                                 t = posixtimer_get_target(tmr)
                                                                 lock_task_sighand()
                                                                    // OBSERVES A
                                                                    if (!list_empty(q))
                                                                       // BUT NOT B
                                                                       list_add_tail(q) // D

Then who knows which write wins, B or D?

Thanks.

-- 
Frederic Weisbecker
SUSE Labs

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

[Thomas Gleixner]

On Mon, Sep 07 2026 at 14:31, Frederic Weisbecker wrote:
> Le Sat, Sep 05, 2026 at 08:59:01PM +0200, Thomas Gleixner a écrit :
>> -out:
>> -	spin_unlock_irq(&tsk->sighand->siglock);
>> +	flush_sigqueue_list(&sigq_list);
>
> It probably doesn't matter in practice, I don't know feel free to ignore,
> but FWIW it looks like it's still vulnerable to the theoretical far fetched
> race I described. The head is moved under the lock but individual nodes are
> deleted without the lock.
>
> CPU 0                                CPU 1                   CPU 2
> -----                                -----                   -----
>
> exit_signals()
>    spin_lock(sighand)
>    tsk->flags |= PF_EXITING;
>    list_splice_init(&queue->list, head);
>    spin_unlock(sighand)
>
>    list_for_each_safe(head, node)
>       list_del_init(node)
>          node->next = node // A
>          node->prev = node // B
> ...
>                                      de_thread()
>                                         // acquired tsk->flags
>                                         // and signal flushed
>                                         // through tasklist_lock
>                                         transfer_pid() // C
>                                                            
>                                                            posix_timer_fn()
>                                                               posixtimer_send_sigqueue()
>                                                                  // OBSERVES C
>                                                                  t = posixtimer_get_target(tmr)
>                                                                  lock_task_sighand()
>                                                                     // OBSERVES A
>                                                                     if (!list_empty(q))
>                                                                        // BUT NOT B
>                                                                        list_add_tail(q) // D
>
> Then who knows which write wins, B or D?

For a moment you almost convinced me, but that's not possible:

    de_thread()
        ....

	if (!thread_leader()) {
            wait_until(old_leader->exit_state);

            transfer_pid();

old_leader sets the exit_state in exit_notify():

    do_exit()
      exit_signals()
        lock(sighand)
          old_leader->flags |= PF_EXITING;
          head = remove_signals()
        unlock(sighand)
        flush_list(head)
     ...
     exit_notify()
       old_leader->exit_state = EXIT_XXX;

From a program order POV the flush is completed _before_ the new leader
can observe old_leader->exit_state and swap TIDS. exit_notify() and the
wait in de_thread() are serialized via tasklist_lock.

The signal is either dropped before transfer_pid() is observable due to
PF_EXITING on the old leader or queued on the new leader and then
discarded in posixtimer_exit() -> flush_itimer_signals().

The only valid question is whether it is guaranteed that on a weakly
ordered system the stores in flush_sigqueue_list() are visible _before_
transfer_pid() is visible to the third party.

It's not obvious of course and might deserve a comment.

      exit_signals()
        lock(sighand)
          old_leader->flags |= PF_EXITING;
          head = remove_signals()
#1      // RELEASE: PF_EXITING must become visible
        unlock(sighand)
        flush_list(head)

     ...
     posixtimer_exit()
       posix_cpu_timers_exit_task()
        lock(sighand)
        ...
#2      // RELEASE: The stores in flush_list() must become visible
        //          They might be already in case of preemption
        //	    or due a RELEASE operation in seccomp_filter_release()
        unlock(sighand)

    ...
    exit_notify()
      lock(task_list_lock)
      exit_state = EXIT_ZOMBIE;
#3    // RELEASE: exit_state must become visible
      unlock(task_list_lock)

So the new leader cannot proceed before #3 which means it can't swap
TIDs before that point. That requires task_list_lock so there is no way
that the TID swap can trickle before the lock is held and exit_state
being non-zero.

Though the important part is that the third party on CPU3 has to acquire
sighand lock in posixtimer_send_sigqueue(), which is an ACQUIRE
operation. That means _all_ accesses to tsk::flags and to the sigqueue
must happen _after_ the lock is acquired.

If it acquires it after #1 and before the TID swap it must observe
PF_EXITING and return immediately. So a concurrent modification of
timer::sigqueue in flush_list() or not-yet visible stores are
irrelevant.

If it acquires it after #2 it must observe the full writes to the
sigqueue. So after that point it does not longer matter whether the PID
resolves to T1 or T2.

No?

Thanks,

        tglx

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

[Frederic Weisbecker]

Le Mon, Sep 07, 2026 at 05:26:04PM +0200, Thomas Gleixner a écrit :
> On Mon, Sep 07 2026 at 14:31, Frederic Weisbecker wrote:
> > Le Sat, Sep 05, 2026 at 08:59:01PM +0200, Thomas Gleixner a écrit :
> >> -out:
> >> -	spin_unlock_irq(&tsk->sighand->siglock);
> >> +	flush_sigqueue_list(&sigq_list);
> >
> > It probably doesn't matter in practice, I don't know feel free to ignore,
> > but FWIW it looks like it's still vulnerable to the theoretical far fetched
> > race I described. The head is moved under the lock but individual nodes are
> > deleted without the lock.
> >
> > CPU 0                                CPU 1                   CPU 2
> > -----                                -----                   -----
> >
> > exit_signals()
> >    spin_lock(sighand)
> >    tsk->flags |= PF_EXITING;
> >    list_splice_init(&queue->list, head);
> >    spin_unlock(sighand)
> >
> >    list_for_each_safe(head, node)
> >       list_del_init(node)
> >          node->next = node // A
> >          node->prev = node // B
> > ...
> >                                      de_thread()
> >                                         // acquired tsk->flags
> >                                         // and signal flushed
> >                                         // through tasklist_lock
> >                                         transfer_pid() // C
> >                                                            
> >                                                            posix_timer_fn()
> >                                                               posixtimer_send_sigqueue()
> >                                                                  // OBSERVES C
> >                                                                  t = posixtimer_get_target(tmr)
> >                                                                  lock_task_sighand()
> >                                                                     // OBSERVES A
> >                                                                     if (!list_empty(q))
> >                                                                        // BUT NOT B
> >                                                                        list_add_tail(q) // D
> >
> > Then who knows which write wins, B or D?
> 
> For a moment you almost convinced me, but that's not possible:
> 
>     de_thread()
>         ....
> 
> 	if (!thread_leader()) {
>             wait_until(old_leader->exit_state);
> 
>             transfer_pid();
> 
> old_leader sets the exit_state in exit_notify():
> 
>     do_exit()
>       exit_signals()
>         lock(sighand)
>           old_leader->flags |= PF_EXITING;
>           head = remove_signals()
>         unlock(sighand)
>         flush_list(head)
>      ...
>      exit_notify()
>        old_leader->exit_state = EXIT_XXX;
> 
> From a program order POV the flush is completed _before_ the new leader
> can observe old_leader->exit_state and swap TIDS. exit_notify() and the
> wait in de_thread() are serialized via tasklist_lock.

Yes on that side all is program order. But the ordering is not mirrored on
the other side (at this stage of the patchset).

> 
> The signal is either dropped before transfer_pid() is observable due to
> PF_EXITING on the old leader or queued on the new leader and then
> discarded in posixtimer_exit() -> flush_itimer_signals().
> 
> The only valid question is whether it is guaranteed that on a weakly
> ordered system the stores in flush_sigqueue_list() are visible _before_
> transfer_pid() is visible to the third party.
> 
> It's not obvious of course and might deserve a comment.
> 
>       exit_signals()
>         lock(sighand)
>           old_leader->flags |= PF_EXITING;
>           head = remove_signals()
> #1      // RELEASE: PF_EXITING must become visible
>         unlock(sighand)
>         flush_list(head)
> 
>      ...
>      posixtimer_exit()
>        posix_cpu_timers_exit_task()
>         lock(sighand)
>         ...
> #2      // RELEASE: The stores in flush_list() must become visible
>         //          They might be already in case of preemption
>         //	    or due a RELEASE operation in seccomp_filter_release()
>         unlock(sighand)

That second step only appears at the end of the patchset, right? Otherwise
it's done on release_task(), which is after transfer_pid().

> 
>     ...
>     exit_notify()
>       lock(task_list_lock)
>       exit_state = EXIT_ZOMBIE;
> #3    // RELEASE: exit_state must become visible
>       unlock(task_list_lock)
> 
> So the new leader cannot proceed before #3 which means it can't swap
> TIDs before that point. That requires task_list_lock so there is no way
> that the TID swap can trickle before the lock is held and exit_state
> being non-zero.
> 
> Though the important part is that the third party on CPU3 has to acquire
> sighand lock in posixtimer_send_sigqueue(), which is an ACQUIRE
> operation. That means _all_ accesses to tsk::flags and to the sigqueue
> must happen _after_ the lock is acquired.
> 
> If it acquires it after #1 and before the TID swap it must observe
> PF_EXITING and return immediately. So a concurrent modification of
> timer::sigqueue in flush_list() or not-yet visible stores are
> irrelevant.
> 
> If it acquires it after #2 it must observe the full writes to the
> sigqueue. So after that point it does not longer matter whether the PID
> resolves to T1 or T2.
> 
> No?

At the end of the patchset yes. But it doesn't look that way in this
very patch which is to be backported alone.

Thanks.

-- 
Frederic Weisbecker
SUSE Labs

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

[Thomas Gleixner]

On Mon, Sep 07 2026 at 22:15, Frederic Weisbecker wrote:
> Le Mon, Sep 07, 2026 at 05:26:04PM +0200, Thomas Gleixner a écrit :
>> It's not obvious of course and might deserve a comment.
>> 
>>       exit_signals()
>>         lock(sighand)
>>           old_leader->flags |= PF_EXITING;
>>           head = remove_signals()
>> #1      // RELEASE: PF_EXITING must become visible
>>         unlock(sighand)
>>         flush_list(head)
>> 
>>      ...
>>      posixtimer_exit()
>>        posix_cpu_timers_exit_task()
>>         lock(sighand)
>>         ...
>> #2      // RELEASE: The stores in flush_list() must become visible
>>         //          They might be already in case of preemption
>>         //	    or due a RELEASE operation in seccomp_filter_release()
>>         unlock(sighand)
>
> That second step only appears at the end of the patchset, right? Otherwise
> it's done on release_task(), which is after transfer_pid().

Cleaning up the enqueued posix CPU timers has nothing to do with the
signals.

>>     exit_notify()
>>       lock(task_list_lock)
>>       exit_state = EXIT_ZOMBIE;
>> #3    // RELEASE: exit_state must become visible

In context of patch 1 alone, this RELEASE operation guarantees that the
stores in flush_list() are visible.

The new leader cannot proceed with swapping the TIDs _before_ it
acquires task list lock and observes under task_list_lock

         old_leader->exit_state != 0

The TID swap cannot be reordered by the CPU _before_ task list lock is
acquired and the exit_state is observed as non-zero.

As the exit_notify() RELEASE made both the exit_state store and the
preceeding flush_list() stores visible the third party must observe them
correctly as well when it can observe the TID swap.

It does not matter whether the RELEASE operation after flush_list() is
spin_unlock(siglock) or any other RELEASE operation before and including
the final one in exit_notify().

Any of them will provide the guarantee because _all_ preceeding stores
must be visible before the RELEASE operation is complete. 

No?

Thanks,

        tglx

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

[Frederic Weisbecker]

Le Tue, Sep 08, 2026 at 12:28:04AM +0200, Thomas Gleixner a écrit :
> On Mon, Sep 07 2026 at 22:15, Frederic Weisbecker wrote:
> > Le Mon, Sep 07, 2026 at 05:26:04PM +0200, Thomas Gleixner a écrit :
> >> It's not obvious of course and might deserve a comment.
> >> 
> >>       exit_signals()
> >>         lock(sighand)
> >>           old_leader->flags |= PF_EXITING;
> >>           head = remove_signals()
> >> #1      // RELEASE: PF_EXITING must become visible
> >>         unlock(sighand)
> >>         flush_list(head)
> >> 
> >>      ...
> >>      posixtimer_exit()
> >>        posix_cpu_timers_exit_task()
> >>         lock(sighand)
> >>         ...
> >> #2      // RELEASE: The stores in flush_list() must become visible
> >>         //          They might be already in case of preemption
> >>         //	    or due a RELEASE operation in seccomp_filter_release()
> >>         unlock(sighand)
> >
> > That second step only appears at the end of the patchset, right? Otherwise
> > it's done on release_task(), which is after transfer_pid().
> 
> Cleaning up the enqueued posix CPU timers has nothing to do with the
> signals.
> 
> >>     exit_notify()
> >>       lock(task_list_lock)
> >>       exit_state = EXIT_ZOMBIE;
> >> #3    // RELEASE: exit_state must become visible
> 
> In context of patch 1 alone, this RELEASE operation guarantees that the
> stores in flush_list() are visible.
> 
> The new leader cannot proceed with swapping the TIDs _before_ it
> acquires task list lock and observes under task_list_lock
> 
>          old_leader->exit_state != 0
> 
> The TID swap cannot be reordered by the CPU _before_ task list lock is
> acquired and the exit_state is observed as non-zero.
> 
> As the exit_notify() RELEASE made both the exit_state store and the
> preceeding flush_list() stores visible the third party must observe them
> correctly as well when it can observe the TID swap.
> 
> It does not matter whether the RELEASE operation after flush_list() is
> spin_unlock(siglock) or any other RELEASE operation before and including
> the final one in exit_notify().
> 
> Any of them will provide the guarantee because _all_ preceeding stores
> must be visible before the RELEASE operation is complete. 
> 
> No?
> 
> Thanks,

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 ?

Thanks.

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

[Oleg Nesterov]

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);
-	}
+}
+
+static void flush_sigqueue_list(struct list_head *head)
+{
+	LIST_HEAD(flush);
+	list_splice_init(head, &flush);
+	__flush_sigqueue_list(&flush);
 }
 
 void flush_sigqueue(struct sigpending *queue)
@@ -3196,7 +3201,7 @@ void exit_signals(struct task_struct *tsk)
 
 	cgroup_threadgroup_change_end(tsk);
 
-	flush_sigqueue_list(&sigq_list);
+	__flush_sigqueue_list(&sigq_list);
 
 	/*
 	 * If group stop has completed, deliver the notification.  This

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

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

[Peter Zijlstra]

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?

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

[Thomas Gleixner]

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.

 do_exit()
   exit_signals()
     LOCK siglock
     tsk->flags |= PF_EXITING
     UNLOCK siglock

So after this point anything which looks at tsk->flags under siglock
will observe PF_EXITING and not touch the sigqueue. Nothing to see here.

> 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.

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.

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

H                                                             READ PID
							      ....
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.

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.

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

#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.

#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.

#C cannot be reordered against #B and #D

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

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

   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.

The important part is that the visibility of #A (sigqueue stores) and #G
(PID store) is fully ordered through task list lock.

So #H _cannot_ observe #G without observing #A - not even on PowerPC or
similar insanities.

No?

Also doing the PID swap under sighand lock is not solving anything
either because posix_timer_send_sigqueue() does the lookup without the
lock simply because it does not know which task it is upfront. So it
would have to redo and validate the lookup with the lock held.

Thanks,

        tglx

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

[Peter Zijlstra]

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.

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

[Peter Zijlstra]

On Wed, Sep 09, 2026 at 11:55:19AM +0200, Peter Zijlstra wrote:
> 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
> 

For those that case: UNLOCK+LOCK is RCsc on all architectures except
Power.

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

[Thomas Gleixner]

On Wed, Sep 09 2026 at 11:55, Peter Zijlstra wrote:
> On Wed, Sep 09, 2026 at 11:08:31AM +0200, Thomas Gleixner wrote:
> 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

Yes. I should have argued with UNLOCK+LOCK instead. My bad.

> 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.

Correct.

> *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

Yes. That was my thinking, but I obviously expressed it incorrectly.

> 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.

Right. I guess it's worth to document that somewhere at least in the
change log of this patch.

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

[Frederic Weisbecker]

Le Wed, Sep 09, 2026 at 11:55:18AM +0200, Peter Zijlstra a écrit :
> 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.

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?

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

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

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

[Peter Zijlstra]

On Wed, Sep 09, 2026 at 02:45:55PM +0200, Peter Zijlstra wrote:
> On Wed, Sep 09, 2026 at 02:13:11PM +0200, Frederic Weisbecker wrote:

> > 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);

There is no dependency between r0 and sighand. While I think there is in
posixtimer_send_sigqueue(). Does making it smp_load_acquire() help?

> > 	// queue signal
> > 	r1 = READ_ONCE(*next);
> > 	if (r1 == 0)
> > 		WRITE_ONCE(*prev, 2);
> > 	spin_unlock(sighand);
> > }
> > 
> > exists (prev=1 /\ 2:r0=1) (* Bad outcome. *)

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

[Thomas Gleixner]

On Wed, Sep 09 2026 at 14:51, Peter Zijlstra wrote:
> On Wed, Sep 09, 2026 at 02:45:55PM +0200, Peter Zijlstra wrote:
>> On Wed, Sep 09, 2026 at 02:13:11PM +0200, Frederic Weisbecker wrote:
>> > P2(int *next, int *prev, int *pid, spinlock_t *sighand)
>> > {
>> > 	int r0;
>> > 	int r1;
>> > 	// get target
>> > 	r0 = READ_ONCE(*pid);
>> > 	spin_lock(sighand);
>
> There is no dependency between r0 and sighand. While I think there is in
> posixtimer_send_sigqueue(). Does making it smp_load_acquire() help?

sighand is r0->sighand->siglock and obviously not known before r0 is
read. So yes there is a data dependency in reality :)

>> > 	// queue signal
>> > 	r1 = READ_ONCE(*next);
>> > 	if (r1 == 0)
>> > 		WRITE_ONCE(*prev, 2);
>> > 	spin_unlock(sighand);
>> > }
>> > 
>> > exists (prev=1 /\ 2:r0=1) (* Bad outcome. *)

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

[Frederic Weisbecker]

Le Wed, Sep 09, 2026 at 03:45:36PM +0200, Thomas Gleixner a écrit :
> On Wed, Sep 09 2026 at 14:51, Peter Zijlstra wrote:
> > On Wed, Sep 09, 2026 at 02:45:55PM +0200, Peter Zijlstra wrote:
> >> On Wed, Sep 09, 2026 at 02:13:11PM +0200, Frederic Weisbecker wrote:
> >> > P2(int *next, int *prev, int *pid, spinlock_t *sighand)
> >> > {
> >> > 	int r0;
> >> > 	int r1;
> >> > 	// get target
> >> > 	r0 = READ_ONCE(*pid);
> >> > 	spin_lock(sighand);
> >
> > There is no dependency between r0 and sighand. While I think there is in
> > posixtimer_send_sigqueue(). Does making it smp_load_acquire() help?
> 
> sighand is r0->sighand->siglock and obviously not known before r0 is
> read. So yes there is a data dependency in reality :)

So unfortunately litmus tests don't support structures. So instead
of transfering the pid, I fake the data dependency by transfering the
sighand directly and then yes it works. I don't know what is the name
of the pattern behind that.

This is not a control dependency as there is no LOAD-cond-store. If
someone can shed some light on this?

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, spinlock_t *tasklist_lock, spinlock_t *sighand, spinlock_t **psighand)
{
	int r0;

	// de_thread()
	spin_lock(tasklist_lock);
	r0 = READ_ONCE(*exit_state);
	if (r0 == 1) {
		// exchange_tids()
		WRITE_ONCE(*psighand, sighand);
	}
	spin_unlock(tasklist_lock);
}

P2(int *next, int *prev, int *pid, spinlock_t *sighand, spinlock_t **psighand)
{
	spinlock_t *r0;
	int r1;

	// get target
	r0 = READ_ONCE(*psighand);
	spin_lock(r0);
	// queue signal
	r1 = READ_ONCE(*next);
	if (r1 == 1) {
		WRITE_ONCE(*prev, 2);
	}
	spin_unlock(r0);
}

exists (prev=1 /\ 2:r0=sighand) (* Bad outcome. *)
---
herd7 -conf linux-kernel.cfg ~/farfetched.litmus 
Test MP+farfetched Allowed
States 3
2:r0=0; [prev]=1;
2:r0=0; [prev]=2;
2:r0=sighand; [prev]=2;
No
Witnesses
Positive: 0 Negative: 7
Condition exists ([prev]=1 /\ 2:r0=sighand)
Observation MP+farfetched Never 0 7

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

[Frederic Weisbecker]

Le Wed, Sep 09, 2026 at 03:45:36PM +0200, Thomas Gleixner a écrit :
> On Wed, Sep 09 2026 at 14:51, Peter Zijlstra wrote:
> > On Wed, Sep 09, 2026 at 02:45:55PM +0200, Peter Zijlstra wrote:
> >> On Wed, Sep 09, 2026 at 02:13:11PM +0200, Frederic Weisbecker wrote:
> >> > P2(int *next, int *prev, int *pid, spinlock_t *sighand)
> >> > {
> >> > 	int r0;
> >> > 	int r1;
> >> > 	// get target
> >> > 	r0 = READ_ONCE(*pid);
> >> > 	spin_lock(sighand);
> >
> > There is no dependency between r0 and sighand. While I think there is in
> > posixtimer_send_sigqueue(). Does making it smp_load_acquire() help?
> 
> sighand is r0->sighand->siglock and obviously not known before r0 is
> read. So yes there is a data dependency in reality :)

Ah but the script writes *psighand on one hand and the spin_lock on the
other hand does an acquire so of course it works. But that doesn't tell
if that's transferrable through struct pid->task write on one hand and
struct pid->task deref on the other hand plus task->sighand dependencies.

My brain melts enough for today...

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

[Alan Stern]

On Wed, Sep 09, 2026 at 02:45:55PM +0200, Peter Zijlstra wrote:
> On Wed, Sep 09, 2026 at 02:13:11PM +0200, Frederic Weisbecker wrote:
> > 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?

You got it right.  The required ordering is that instructions before 
(and on the same CPU as) the unlock are ordered before instructions 
after (and on the same CPU as) the lock for read->read, read->write, and 
write->write cases, but not for write->read cases.  Furthermore, this 
ordering requirement applies from the point of view of all CPUs, whether 
they access the lock variables or not.  In other words, the ordering is 
required to be RCtso.

This ordering is weaker than full smp_mb() in two respects:

	As mentioned above, it doesn't order earlier writes against
	later reads;

	In the WRITE->WRITE case, the guarantee is only that each CPU
	will observe the first write before it observes the second.
	There is no guarantee that _every_ CPU will observe the first
	write before _any_ of them observe the second.

Alan Stern

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

[Frederic Weisbecker]

Le Wed, Sep 09, 2026 at 02:45:55PM +0200, Peter Zijlstra a écrit :
> 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")

I didn't know that UNLOCK+LOCK can pair with smp_load_acquire(). Good to know.

But unlock+lock doesn't pair with unlock+lock on different CPU.

C MP+polocks

{}

P0(int *A, int *B, spinlock_t *mylock)
{
	spin_lock(mylock);
	WRITE_ONCE(*A, 1);
	spin_unlock(mylock);
	spin_lock(mylock);
	WRITE_ONCE(*B, 1);
	spin_unlock(mylock);
}

P1(int *A, int *B, spinlock_t *otherlock)
{
	int r0;
	int r1;

	r0 = READ_ONCE(*B);
	spin_lock(otherlock);
	r1 = READ_ONCE(*A);
	spin_unlock(otherlock);
}

exists (1:r0=1 /\ 1:r1=0) (* Bad outcome happens *)

But yeah there is no data dependency involved. Let me answer to
that to tglx.

-- 
Frederic Weisbecker
SUSE Labs

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

[Alan Stern]

On Wed, Sep 09, 2026 at 04:45:15PM +0200, Frederic Weisbecker wrote:
> I didn't know that UNLOCK+LOCK can pair with smp_load_acquire(). Good to know.

I don't know what that means.  Regardless, LOCK doesn't pair with 
smp_load_acquire().  UNLOCK does, but only to the extent that acts as a 
release.

> But unlock+lock doesn't pair with unlock+lock on different CPU.
> 
> C MP+polocks
> 
> {}
> 
> P0(int *A, int *B, spinlock_t *mylock)
> {
> 	spin_lock(mylock);
> 	WRITE_ONCE(*A, 1);
> 	spin_unlock(mylock);
> 	spin_lock(mylock);
> 	WRITE_ONCE(*B, 1);
> 	spin_unlock(mylock);
> }
> 
> P1(int *A, int *B, spinlock_t *otherlock)
> {
> 	int r0;
> 	int r1;
> 
> 	r0 = READ_ONCE(*B);
> 	spin_lock(otherlock);
> 	r1 = READ_ONCE(*A);
> 	spin_unlock(otherlock);
> }
> 
> exists (1:r0=1 /\ 1:r1=0) (* Bad outcome happens *)

I can't tell what you're trying to do here.  The UNLOCK-LOCK ordering 
in P0 means that P1 sees A=1 before it sees B=1.  But nothing in this 
litmus test forces P1 to execute READ_ONCE(*b) before READ_ONCE(*A).  If 
the reads are executed in the opposite order, you can see how P1 might 
get r0=1 and r1=0.

If P1 had done this instead:

P1(int *A, int *B)
{
	int r0;
	int r1;

	r0 = READ_ONCE(*B);
	smp_rmb();
	r1 = READ_ONCE(*A);
}

then r0=1 and r1=0 would be impossible.

Alan Stern

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

[Thomas Gleixner]

On Wed, Sep 09 2026 at 15:28, Alan Stern wrote:
> On Wed, Sep 09, 2026 at 04:45:15PM +0200, Frederic Weisbecker wrote:
> I can't tell what you're trying to do here.  The UNLOCK-LOCK ordering 
> in P0 means that P1 sees A=1 before it sees B=1.  But nothing in this 
> litmus test forces P1 to execute READ_ONCE(*b) before READ_ONCE(*A).  If 
> the reads are executed in the opposite order, you can see how P1 might 
> get r0=1 and r1=0.

The problem we are debating is:

    C = VAL1

    CPU0                    CPU1		CPU2
    
    STORE(A0, 0)
    STORE(A1, 0)

    LOCK(TLOCK)
    STORE(B, 1) // 0 -> 1
    UNLOCK(TLOCK)

                           LOCK(TLOCK)
                           b = LOAD(B)
                           if (b)
                              STORE(C, VAL0)

                                                c = LOAD(C)
                                                LOCK(c->lock)
                                                  a0 = LOAD(A0)
                                                  if (!a0)
                                                     STORE(A0, X1)
                                                     STORE(A1, X2)

The question is whether CPU2 can observe C == VAL0 and A0 == NULL before
A1 has completed.

My and Peter's argument is that the sequence

   UNLOCK(TLOCK) on CPU0   -> LOCK(TLOCK) on CPU1

implies RCtso and therefore the stores to A0 and A1 on CPU0 must be
before the store to C on CPU1.

Now because the LOAD(C) on CPU2 depends on that STORE(C) the
LOCK(c->lock) ensures that LOAD(A0) can't be reordered and because of
that STORE(A1, 0) has completed before that.

CPU2 LOAD(C) observing VAL0 has a data dependency on the STORE(C, VAL0)
on CPU1, which as argued above can only happen after the UNLOCK/LOCK
sequence CPU1 observes the STORE(B).

Subsequently LOCK(c->lock) has a data dependency on LOAD(C) and the LOCK
operation prevents that LOAD(A0) can be reordered before LOCK(c->lock).

So despite the fact that c->lock != TLOCK the UNLOCK(TLOCK)/LOCK(TLOCK)
sequence, which implies RCtso, the following takes care of it:

    1) the data dependency between the STORE(C, VAL0) on CPU1 and the
       c = LOAD(C) on CPU2 observing VAL0
   
    2) the data dependency of LOCK(c->lock) on #1

    3) due to LOCK() in #2 LOAD(A0) cannot observe the STORE(A0, 0) on
       CPU0 without the STORE(A1, 0) on CPU0 has completed.

If #3 can happen then that would obviously cause undebuggable data
corruption.

I hope this is understandable enough despite my brain having melted
several times by now while writing it up.

Thanks,

        tglx

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

[Alan Stern]

On Wed, Sep 09, 2026 at 10:49:30PM +0200, Thomas Gleixner wrote:
> On Wed, Sep 09 2026 at 15:28, Alan Stern wrote:
> > On Wed, Sep 09, 2026 at 04:45:15PM +0200, Frederic Weisbecker wrote:
> > I can't tell what you're trying to do here.  The UNLOCK-LOCK ordering 
> > in P0 means that P1 sees A=1 before it sees B=1.  But nothing in this 
> > litmus test forces P1 to execute READ_ONCE(*b) before READ_ONCE(*A).  If 
> > the reads are executed in the opposite order, you can see how P1 might 
> > get r0=1 and r1=0.
> 
> The problem we are debating is:
> 
>     C = VAL1
> 
>     CPU0                    CPU1		CPU2
>     
>     STORE(A0, 0)
>     STORE(A1, 0)
> 
>     LOCK(TLOCK)
>     STORE(B, 1) // 0 -> 1
>     UNLOCK(TLOCK)
> 
>                            LOCK(TLOCK)
>                            b = LOAD(B)
>                            if (b)
>                               STORE(C, VAL0)
> 
>                                                 c = LOAD(C)
>                                                 LOCK(c->lock)
>                                                   a0 = LOAD(A0)
>                                                   if (!a0)
>                                                      STORE(A0, X1)
>                                                      STORE(A1, X2)
> 
> The question is whether CPU2 can observe C == VAL0 and A0 == NULL before
> A1 has completed.
> 
> My and Peter's argument is that the sequence
> 
>    UNLOCK(TLOCK) on CPU0   -> LOCK(TLOCK) on CPU1
> 
> implies RCtso and therefore the stores to A0 and A1 on CPU0 must be
> before the store to C on CPU1.
> 
> Now because the LOAD(C) on CPU2 depends on that STORE(C) the
> LOCK(c->lock) ensures that LOAD(A0) can't be reordered and because of
> that STORE(A1, 0) has completed before that.
> 
> CPU2 LOAD(C) observing VAL0 has a data dependency on the STORE(C, VAL0)
> on CPU1, which as argued above can only happen after the UNLOCK/LOCK
> sequence CPU1 observes the STORE(B).
> 
> Subsequently LOCK(c->lock) has a data dependency on LOAD(C) and the LOCK
> operation prevents that LOAD(A0) can be reordered before LOCK(c->lock).
> 
> So despite the fact that c->lock != TLOCK the UNLOCK(TLOCK)/LOCK(TLOCK)
> sequence, which implies RCtso, the following takes care of it:
> 
>     1) the data dependency between the STORE(C, VAL0) on CPU1 and the
>        c = LOAD(C) on CPU2 observing VAL0
>    
>     2) the data dependency of LOCK(c->lock) on #1
> 
>     3) due to LOCK() in #2 LOAD(A0) cannot observe the STORE(A0, 0) on
>        CPU0 without the STORE(A1, 0) on CPU0 has completed.
> 
> If #3 can happen then that would obviously cause undebuggable data
> corruption.
> 
> I hope this is understandable enough despite my brain having melted
> several times by now while writing it up.

I see.  Yes, your analysis is right.  And Frederic's latest LKML litmus 
test confirms the result.

Alan Stern

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

[Frederic Weisbecker]

Le Wed, Sep 09, 2026 at 05:11:27PM -0400, Alan Stern a écrit :
> On Wed, Sep 09, 2026 at 10:49:30PM +0200, Thomas Gleixner wrote:
> > On Wed, Sep 09 2026 at 15:28, Alan Stern wrote:
> > > On Wed, Sep 09, 2026 at 04:45:15PM +0200, Frederic Weisbecker wrote:
> > > I can't tell what you're trying to do here.  The UNLOCK-LOCK ordering 
> > > in P0 means that P1 sees A=1 before it sees B=1.  But nothing in this 
> > > litmus test forces P1 to execute READ_ONCE(*b) before READ_ONCE(*A).  If 
> > > the reads are executed in the opposite order, you can see how P1 might 
> > > get r0=1 and r1=0.
> > 
> > The problem we are debating is:
> > 
> >     C = VAL1
> > 
> >     CPU0                    CPU1		CPU2
> >     
> >     STORE(A0, 0)
> >     STORE(A1, 0)
> > 
> >     LOCK(TLOCK)
> >     STORE(B, 1) // 0 -> 1
> >     UNLOCK(TLOCK)
> > 
> >                            LOCK(TLOCK)
> >                            b = LOAD(B)
> >                            if (b)
> >                               STORE(C, VAL0)
> > 
> >                                                 c = LOAD(C)
> >                                                 LOCK(c->lock)
> >                                                   a0 = LOAD(A0)
> >                                                   if (!a0)
> >                                                      STORE(A0, X1)
> >                                                      STORE(A1, X2)
> > 
> > The question is whether CPU2 can observe C == VAL0 and A0 == NULL before
> > A1 has completed.
> > 
> > My and Peter's argument is that the sequence
> > 
> >    UNLOCK(TLOCK) on CPU0   -> LOCK(TLOCK) on CPU1
> > 
> > implies RCtso and therefore the stores to A0 and A1 on CPU0 must be
> > before the store to C on CPU1.
> > 
> > Now because the LOAD(C) on CPU2 depends on that STORE(C) the
> > LOCK(c->lock) ensures that LOAD(A0) can't be reordered and because of
> > that STORE(A1, 0) has completed before that.
> > 
> > CPU2 LOAD(C) observing VAL0 has a data dependency on the STORE(C, VAL0)
> > on CPU1, which as argued above can only happen after the UNLOCK/LOCK
> > sequence CPU1 observes the STORE(B).
> > 
> > Subsequently LOCK(c->lock) has a data dependency on LOAD(C) and the LOCK
> > operation prevents that LOAD(A0) can be reordered before LOCK(c->lock).
> > 
> > So despite the fact that c->lock != TLOCK the UNLOCK(TLOCK)/LOCK(TLOCK)
> > sequence, which implies RCtso, the following takes care of it:
> > 
> >     1) the data dependency between the STORE(C, VAL0) on CPU1 and the
> >        c = LOAD(C) on CPU2 observing VAL0
> >    
> >     2) the data dependency of LOCK(c->lock) on #1
> > 
> >     3) due to LOCK() in #2 LOAD(A0) cannot observe the STORE(A0, 0) on
> >        CPU0 without the STORE(A1, 0) on CPU0 has completed.
> > 
> > If #3 can happen then that would obviously cause undebuggable data
> > corruption.
> > 
> > I hope this is understandable enough despite my brain having melted
> > several times by now while writing it up.
> 
> I see.  Yes, your analysis is right.  And Frederic's latest LKML litmus 
> test confirms the result.

Alan, let me ask you something, because I'm the only one here puzzled by
this data dependency.

The following scenario works (the bad outcome never happens) because
UNLOCK+LOCK pairs with smp_load_acquire():

	C MP+polocks

	{}

	P0(int *A, int *B, spinlock_t *mylock)
	{
		spin_lock(mylock);
		WRITE_ONCE(*A, 1);
		spin_unlock(mylock);
		spin_lock(mylock);
		WRITE_ONCE(*B, 1);
		spin_unlock(mylock);
	}

	P1(int *A, int *B)
	{
		int r0;
		int r1;

		r0 = smp_load_acquire(B);
		r1 = READ_ONCE(*A);
	}

	exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)


So I understand this one. Now unfortunately litmus doesn't support
structures, but let's suppose it could. I'm taking the previous script
and introduce a small change in P1:

	C MP+polocks

	{}

	P0(int *A, int *B, spinlock_t *mylock)
	{
		spin_lock(mylock);
		WRITE_ONCE(*A, 1);
		spin_unlock(mylock);
		spin_lock(mylock);
		WRITE_ONCE(*B, 1);
		spin_unlock(mylock);
	}

	P1(int *A, int *B)
	{
		int r0;
		int r1

		r0 = READ_ONCE(*B);
		spin_lock(r0->somelock)
		r1 = READ_ONCE(*A);
		spin_unlock(r0->somelock)
	}

	exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)


So instead of doing a LOAD-ACQUIRE on B, I do a plain READ but I also
do a spin_lock right after on a data that depends on that READ. I can't
run that on litmus but this is the same (simplified) pattern as what we
had in this discussion and therefore I assume that it also works (ie: the
bad outcome shouldn't happen), is that right?

Would it also work if spin_lock() was just a LOAD-ACQUIRE?

Does it mean that data dependency implies sufficient ordering such that
a LOAD-ACQUIRE to a data that depends on B provides the same guarantees as
a LOAD-ACQUIRE to B?

The reason I'm asking that is because, unlike control dependency, data
dependency and its guarantees are not well documented. It is defined in
tools/memory-model/Documentation/explanation.txt but not really described
in Documentation/memory-barriers.txt. There is a mention in a scenario within
the section "MULTICOPY ATOMICITY" just to show that it's not as strong as
a full memory barrier.

So if data dependency can provide the guarantee above in my second script
but it's not as strong as a full barrier, this suggests that data dependencies
have their own specific properties that should probably be documentated.

Thanks.

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

[Peter Zijlstra]

On Thu, Sep 10, 2026 at 03:21:54PM +0200, Frederic Weisbecker wrote:

> So I understand this one. Now unfortunately litmus doesn't support
> structures, but let's suppose it could. I'm taking the previous script
> and introduce a small change in P1:
> 
> 	C MP+polocks
> 
> 	{}
> 
> 	P0(int *A, int *B, spinlock_t *mylock)
> 	{
> 		spin_lock(mylock);
> 		WRITE_ONCE(*A, 1);
> 		spin_unlock(mylock);
> 		spin_lock(mylock);
> 		WRITE_ONCE(*B, 1);
> 		spin_unlock(mylock);
> 	}
> 
> 	P1(int *A, int *B)
> 	{
> 		int r0;
> 		int r1
> 
> 		r0 = READ_ONCE(*B);
> 		spin_lock(r0->somelock)
> 		r1 = READ_ONCE(*A);
> 		spin_unlock(r0->somelock)
> 	}
> 
> 	exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)
> 
> 
> So instead of doing a LOAD-ACQUIRE on B, I do a plain READ but I also
> do a spin_lock right after on a data that depends on that READ. I can't
> run that on litmus but this is the same (simplified) pattern as what we
> had in this discussion and therefore I assume that it also works (ie: the
> bad outcome shouldn't happen), is that right?
> 
> Would it also work if spin_lock() was just a LOAD-ACQUIRE?

Yes, see below.

> Does it mean that data dependency implies sufficient ordering such that
> a LOAD-ACQUIRE to a data that depends on B provides the same guarantees as
> a LOAD-ACQUIRE to B?

The way data dependencies work in this case is a LOAD->LOAD ordering.
The LOAD-ACQUIRE that is part of LOCK must happen after the initial
load. And then the later load is constrained by the ACQUIRE.

So LOAD(B) must resolve in order to do LOAD_ACQUIRE(B->lock.value).

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

[Frederic Weisbecker]

Le Thu, Sep 10, 2026 at 03:28:43PM +0200, Peter Zijlstra a écrit :
> On Thu, Sep 10, 2026 at 03:21:54PM +0200, Frederic Weisbecker wrote:
> 
> > So I understand this one. Now unfortunately litmus doesn't support
> > structures, but let's suppose it could. I'm taking the previous script
> > and introduce a small change in P1:
> > 
> > 	C MP+polocks
> > 
> > 	{}
> > 
> > 	P0(int *A, int *B, spinlock_t *mylock)
> > 	{
> > 		spin_lock(mylock);
> > 		WRITE_ONCE(*A, 1);
> > 		spin_unlock(mylock);
> > 		spin_lock(mylock);
> > 		WRITE_ONCE(*B, 1);
> > 		spin_unlock(mylock);
> > 	}
> > 
> > 	P1(int *A, int *B)
> > 	{
> > 		int r0;
> > 		int r1
> > 
> > 		r0 = READ_ONCE(*B);
> > 		spin_lock(r0->somelock)
> > 		r1 = READ_ONCE(*A);
> > 		spin_unlock(r0->somelock)
> > 	}
> > 
> > 	exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)
> > 
> > 
> > So instead of doing a LOAD-ACQUIRE on B, I do a plain READ but I also
> > do a spin_lock right after on a data that depends on that READ. I can't
> > run that on litmus but this is the same (simplified) pattern as what we
> > had in this discussion and therefore I assume that it also works (ie: the
> > bad outcome shouldn't happen), is that right?
> > 
> > Would it also work if spin_lock() was just a LOAD-ACQUIRE?
> 
> Yes, see below.
> 
> > Does it mean that data dependency implies sufficient ordering such that
> > a LOAD-ACQUIRE to a data that depends on B provides the same guarantees as
> > a LOAD-ACQUIRE to B?
> 
> The way data dependencies work in this case is a LOAD->LOAD ordering.
> The LOAD-ACQUIRE that is part of LOCK must happen after the initial
> load. And then the later load is constrained by the ACQUIRE.
> 
> So LOAD(B) must resolve in order to do LOAD_ACQUIRE(B->lock.value).

Ok I must confess that's not a pattern I'm used to and therefore it's
still a bit counter-intuitive to me. I hope we can document that somehow
somewhere.

Thanks a lot for the explanation!

-- 
Frederic Weisbecker
SUSE Labs

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

[Peter Zijlstra]

On Fri, Sep 11, 2026 at 11:58:04AM +0200, Frederic Weisbecker wrote:
> Le Thu, Sep 10, 2026 at 03:28:43PM +0200, Peter Zijlstra a écrit :

> > The way data dependencies work in this case is a LOAD->LOAD ordering.
> > The LOAD-ACQUIRE that is part of LOCK must happen after the initial
> > load. And then the later load is constrained by the ACQUIRE.
> > 
> > So LOAD(B) must resolve in order to do LOAD_ACQUIRE(B->lock.value).
> 
> Ok I must confess that's not a pattern I'm used to and therefore it's
> still a bit counter-intuitive to me. I hope we can document that somehow
> somewhere.

One way of looking at it is the computation for the address of
&B->lock.value.

A	void *r = LOAD(B);			 // B
B	r += offsetof(typeof(*B), lock);	 // &B->lock
	spin_lock(r)
C	  r += offsetof(typeof(B->lock), value); // &B->lock.value
	  for (;;)
D	    v = LOAD_ACQUIRE(r);
	    if (!v && !cmpxchg_relaxed(r, 0, 1))
	      break;
	    cpu_relax();

Note how the LOAD_ACQUIRE() at D depends on the computation of C, and B,
which in turn depend on the load of A.

If A does not complete, B,C cannot compute the address for the load of
D. Therefore A must come before D.

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

[Frederic Weisbecker]

Le Fri, Sep 11, 2026 at 12:22:03PM +0200, Peter Zijlstra a écrit :
> On Fri, Sep 11, 2026 at 11:58:04AM +0200, Frederic Weisbecker wrote:
> > Le Thu, Sep 10, 2026 at 03:28:43PM +0200, Peter Zijlstra a écrit :
> 
> > > The way data dependencies work in this case is a LOAD->LOAD ordering.
> > > The LOAD-ACQUIRE that is part of LOCK must happen after the initial
> > > load. And then the later load is constrained by the ACQUIRE.
> > > 
> > > So LOAD(B) must resolve in order to do LOAD_ACQUIRE(B->lock.value).
> > 
> > Ok I must confess that's not a pattern I'm used to and therefore it's
> > still a bit counter-intuitive to me. I hope we can document that somehow
> > somewhere.
> 
> One way of looking at it is the computation for the address of
> &B->lock.value.
> 
> A	void *r = LOAD(B);			 // B
> B	r += offsetof(typeof(*B), lock);	 // &B->lock
> 	spin_lock(r)
> C	  r += offsetof(typeof(B->lock), value); // &B->lock.value
> 	  for (;;)
> D	    v = LOAD_ACQUIRE(r);
> 	    if (!v && !cmpxchg_relaxed(r, 0, 1))
> 	      break;
> 	    cpu_relax();
> 
> Note how the LOAD_ACQUIRE() at D depends on the computation of C, and B,
> which in turn depend on the load of A.
> 
> If A does not complete, B,C cannot compute the address for the load of
> D. Therefore A must come before D.

Right, it's just that acquire is often described as ordering access after
a single memory target (matching release before that same single memory target).
But I'm discovering that this property also applies to a memory target that
depends on an actual target, so to speak :o)

Anyway yes, I get the idea now, thanks for the patient explanations!

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

[Alan Stern]

On Fri, Sep 11, 2026 at 12:22:03PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 11, 2026 at 11:58:04AM +0200, Frederic Weisbecker wrote:
> > Le Thu, Sep 10, 2026 at 03:28:43PM +0200, Peter Zijlstra a écrit :
> 
> > > The way data dependencies work in this case is a LOAD->LOAD ordering.
> > > The LOAD-ACQUIRE that is part of LOCK must happen after the initial
> > > load. And then the later load is constrained by the ACQUIRE.
> > > 
> > > So LOAD(B) must resolve in order to do LOAD_ACQUIRE(B->lock.value).
> > 
> > Ok I must confess that's not a pattern I'm used to and therefore it's
> > still a bit counter-intuitive to me. I hope we can document that somehow
> > somewhere.
> 
> One way of looking at it is the computation for the address of
> &B->lock.value.
> 
> A	void *r = LOAD(B);			 // B
> B	r += offsetof(typeof(*B), lock);	 // &B->lock
> 	spin_lock(r)
> C	  r += offsetof(typeof(B->lock), value); // &B->lock.value
> 	  for (;;)
> D	    v = LOAD_ACQUIRE(r);
> 	    if (!v && !cmpxchg_relaxed(r, 0, 1))
> 	      break;
> 	    cpu_relax();
> 
> Note how the LOAD_ACQUIRE() at D depends on the computation of C, and B,
> which in turn depend on the load of A.
> 
> If A does not complete, B,C cannot compute the address for the load of
> D. Therefore A must come before D.

There's one subtle point worth making here, although it doesn't affect 
the final result.

We have to be a little careful when talking about when a load happens, 
because executing a load actually involves two separate things:

	Obtaining a value from memory or cache, and

	Completing (retiring) the load instruction.

These two things don't have to happen at the same time, and in the case 
of speculative loads they can happen at very different times.  So 
whenever you talk about when a load occurs, or whether it occurs before 
or after some other operation, you have to be careful about which part 
of the load you're referring to.

In theory, a CPU could try to obtain a value from memory speculatively 
before it knew for certain what address to load from.  This would defeat 
an address dependency leading to that load.  As mentioned in 
explanation.txt, none of the archictectures supported by Linux do this, 
and the LKMM does not allow it.

However, if it did happen in the example above, the CPU could read the 
value of *r at D before A had finished (say, by remembering what address 
it had used the last time it executed D's load and trying to read 
speculatively from that old address).  But even if this did happen, the 
cmpxchg_relaxed() in the line following D would not be able to execute 
successfully until the CPU knew for certain what the value of *r was, 
that is, until A had completed.

Alan Stern

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

[Alan Stern]

On Thu, Sep 10, 2026 at 03:21:54PM +0200, Frederic Weisbecker wrote:
> Alan, let me ask you something, because I'm the only one here puzzled by
> this data dependency.
> 
> The following scenario works (the bad outcome never happens) because
> UNLOCK+LOCK pairs with smp_load_acquire():
> 
> 	C MP+polocks
> 
> 	{}
> 
> 	P0(int *A, int *B, spinlock_t *mylock)
> 	{
> 		spin_lock(mylock);
> 		WRITE_ONCE(*A, 1);
> 		spin_unlock(mylock);
> 		spin_lock(mylock);
> 		WRITE_ONCE(*B, 1);
> 		spin_unlock(mylock);
> 	}
> 
> 	P1(int *A, int *B)
> 	{
> 		int r0;
> 		int r1;
> 
> 		r0 = smp_load_acquire(B);
> 		r1 = READ_ONCE(*A);
> 	}
> 
> 	exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)
> 
> 
> So I understand this one. Now unfortunately litmus doesn't support
> structures, but let's suppose it could. I'm taking the previous script
> and introduce a small change in P1:
> 
> 	C MP+polocks
> 
> 	{}
> 
> 	P0(int *A, int *B, spinlock_t *mylock)
> 	{
> 		spin_lock(mylock);
> 		WRITE_ONCE(*A, 1);
> 		spin_unlock(mylock);
> 		spin_lock(mylock);
> 		WRITE_ONCE(*B, 1);
> 		spin_unlock(mylock);
> 	}
> 
> 	P1(int *A, int *B)
> 	{
> 		int r0;
> 		int r1
> 
> 		r0 = READ_ONCE(*B);
> 		spin_lock(r0->somelock)
> 		r1 = READ_ONCE(*A);
> 		spin_unlock(r0->somelock)
> 	}
> 
> 	exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)
> 
> 
> So instead of doing a LOAD-ACQUIRE on B, I do a plain READ but I also
> do a spin_lock right after on a data that depends on that READ. I can't
> run that on litmus but this is the same (simplified) pattern as what we
> had in this discussion and therefore I assume that it also works (ie: the
> bad outcome shouldn't happen), is that right?

Yes.

> Would it also work if spin_lock() was just a LOAD-ACQUIRE?

Yes.

> Does it mean that data dependency implies sufficient ordering such that
> a LOAD-ACQUIRE to a data that depends on B provides the same guarantees as
> a LOAD-ACQUIRE to B?

Indeed it does, with the obvious exception that a load-acquire of B 
also provides ordering to any statements in between it and the load of 
the data depending on B.  That is:

	r0 = smp_load_acquire(B);
	X;
	r1 = READ_ONCE(r0->A);

orders the load from B before everything that follows, including X, 
whereas:

	r0 = READ_ONCE(B);
	X;
	r1 = smp_load_acquire(r0->A);

orders the load from B before the load from r0->A and everything 
following it, but not before X.

> The reason I'm asking that is because, unlike control dependency, data
> dependency and its guarantees are not well documented. It is defined in
> tools/memory-model/Documentation/explanation.txt but not really described
> in Documentation/memory-barriers.txt. There is a mention in a scenario within
> the section "MULTICOPY ATOMICITY" just to show that it's not as strong as
> a full memory barrier.
> 
> So if data dependency can provide the guarantee above in my second script
> but it's not as strong as a full barrier, this suggests that data dependencies
> have their own specific properties that should probably be documentated.

Perhaps so.  Can you suggest a place in explanations.txt that could be 
improved?

Here's how I think about ordering guarantees in general.  Not in terms 
of pairing of memory barriers, since (as you pointed out) dependencies 
aren't memory barriers, and also since ordering cycles can involve more 
than two CPUs (so triples or higher, not just pairs).

Instead there's a hierarchy of ordering classes.  The lowest level only 
orders events on a single CPU; it includes dependencies, smp_rmb(), and 
load-acquires.

The next level orders cross-CPU events (i.e., writes), but only in a way 
that affects two CPUs at a time.  It includes things like smp_wmb() and 
store-releases, and it guarantees that if CPU 1 writes A first and B 
second, then CPU 2 will observe the store to A before it observes the 
store to B.  Likewise for CPU 3, CPU 4, etc., but there is no guarantee 
about the order in which differing CPUs will observe the stores.

The highest level orders events in a way that involves all CPUs.  It 
includes things like smp_mb() and synchronize_rcu(), and it says that if 
CPU 1 writes A first and B second, then _every_ CPU will observe the 
store to A before _any_ CPU (including CPU 1!) observes the store to B.

This is a little imprecise, and there are varying details within the 
levels, but the overall idea is basically right.

At any rate, the point you're raising is that dependencies and 
load-acquires both sit at the lowest level of this hierarchy, so they 
provide pretty much the same ordering guarantees.

Alan Stern

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

[Frederic Weisbecker]

Le Thu, Sep 10, 2026 at 11:26:38AM -0400, Alan Stern a écrit :
> On Thu, Sep 10, 2026 at 03:21:54PM +0200, Frederic Weisbecker wrote:
> > Alan, let me ask you something, because I'm the only one here puzzled by
> > this data dependency.
> > 
> > The following scenario works (the bad outcome never happens) because
> > UNLOCK+LOCK pairs with smp_load_acquire():
> > 
> > 	C MP+polocks
> > 
> > 	{}
> > 
> > 	P0(int *A, int *B, spinlock_t *mylock)
> > 	{
> > 		spin_lock(mylock);
> > 		WRITE_ONCE(*A, 1);
> > 		spin_unlock(mylock);
> > 		spin_lock(mylock);
> > 		WRITE_ONCE(*B, 1);
> > 		spin_unlock(mylock);
> > 	}
> > 
> > 	P1(int *A, int *B)
> > 	{
> > 		int r0;
> > 		int r1;
> > 
> > 		r0 = smp_load_acquire(B);
> > 		r1 = READ_ONCE(*A);
> > 	}
> > 
> > 	exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)
> > 
> > 
> > So I understand this one. Now unfortunately litmus doesn't support
> > structures, but let's suppose it could. I'm taking the previous script
> > and introduce a small change in P1:
> > 
> > 	C MP+polocks
> > 
> > 	{}
> > 
> > 	P0(int *A, int *B, spinlock_t *mylock)
> > 	{
> > 		spin_lock(mylock);
> > 		WRITE_ONCE(*A, 1);
> > 		spin_unlock(mylock);
> > 		spin_lock(mylock);
> > 		WRITE_ONCE(*B, 1);
> > 		spin_unlock(mylock);
> > 	}
> > 
> > 	P1(int *A, int *B)
> > 	{
> > 		int r0;
> > 		int r1
> > 
> > 		r0 = READ_ONCE(*B);
> > 		spin_lock(r0->somelock)
> > 		r1 = READ_ONCE(*A);
> > 		spin_unlock(r0->somelock)
> > 	}
> > 
> > 	exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)
> > 
> > 
> > So instead of doing a LOAD-ACQUIRE on B, I do a plain READ but I also
> > do a spin_lock right after on a data that depends on that READ. I can't
> > run that on litmus but this is the same (simplified) pattern as what we
> > had in this discussion and therefore I assume that it also works (ie: the
> > bad outcome shouldn't happen), is that right?
> 
> Yes.
> 
> > Would it also work if spin_lock() was just a LOAD-ACQUIRE?
> 
> Yes.
> 
> > Does it mean that data dependency implies sufficient ordering such that
> > a LOAD-ACQUIRE to a data that depends on B provides the same guarantees as
> > a LOAD-ACQUIRE to B?
> 
> Indeed it does, with the obvious exception that a load-acquire of B 
> also provides ordering to any statements in between it and the load of 
> the data depending on B.  That is:
> 
> 	r0 = smp_load_acquire(B);
> 	X;
> 	r1 = READ_ONCE(r0->A);
> 
> orders the load from B before everything that follows, including X, 
> whereas:
> 
> 	r0 = READ_ONCE(B);
> 	X;
> 	r1 = smp_load_acquire(r0->A);
> 
> orders the load from B before the load from r0->A and everything 
> following it, but not before X.

Ok that matches my understanding.

> 
> > The reason I'm asking that is because, unlike control dependency, data
> > dependency and its guarantees are not well documented. It is defined in
> > tools/memory-model/Documentation/explanation.txt but not really described
> > in Documentation/memory-barriers.txt. There is a mention in a scenario within
> > the section "MULTICOPY ATOMICITY" just to show that it's not as strong as
> > a full memory barrier.
> > 
> > So if data dependency can provide the guarantee above in my second script
> > but it's not as strong as a full barrier, this suggests that data dependencies
> > have their own specific properties that should probably be documentated.
> 
> Perhaps so.  Can you suggest a place in explanations.txt that could be 
> improved?

So in explanations.txt, the different kinds of dependencies are defined
without diving much into properties.

Properties of ordering enforcement tools are typically described in
Documentation/memory-barriers.txt. Control dependencies has its own
section and I suspect there is enough to say about data dependencies
to deserve its own section there.

What I would love to see documented for example is our case: acquire semantics,
which are described to apply one-way from a single memory target, are also
transferrable to other memory targets when there is a data dependency
involved between them.

> Here's how I think about ordering guarantees in general.  Not in terms 
> of pairing of memory barriers, since (as you pointed out) dependencies 
> aren't memory barriers, and also since ordering cycles can involve more 
> than two CPUs (so triples or higher, not just pairs).
> 
> Instead there's a hierarchy of ordering classes.  The lowest level only 
> orders events on a single CPU; it includes dependencies, smp_rmb(), and 
> load-acquires.
> 
> The next level orders cross-CPU events (i.e., writes), but only in a way 
> that affects two CPUs at a time.  It includes things like smp_wmb() and 
> store-releases, and it guarantees that if CPU 1 writes A first and B 
> second, then CPU 2 will observe the store to A before it observes the 
> store to B.  Likewise for CPU 3, CPU 4, etc., but there is no guarantee 
> about the order in which differing CPUs will observe the stores.
> 
> The highest level orders events in a way that involves all CPUs.  It 
> includes things like smp_mb() and synchronize_rcu(), and it says that if 
> CPU 1 writes A first and B second, then _every_ CPU will observe the 
> store to A before _any_ CPU (including CPU 1!) observes the store to B.
> 
> This is a little imprecise, and there are varying details within the 
> levels, but the overall idea is basically right.

Interesting way of seeing it and I *think* I understand :-)

> 
> At any rate, the point you're raising is that dependencies and 
> load-acquires both sit at the lowest level of this hierarchy, so they 
> provide pretty much the same ordering guarantees.
> 
> Alan Stern

Got it.

Thanks a lot for taking the time to explain me all that!

-- 
Frederic Weisbecker
SUSE Labs

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

[Alan Stern]

On Fri, Sep 11, 2026 at 02:27:56PM +0200, Frederic Weisbecker wrote:
> What I would love to see documented for example is our case: acquire semantics,
> which are described to apply one-way from a single memory target, are also
> transferrable to other memory targets when there is a data dependency
> involved between them.

"Transferrable" is not the word I would use, nor does it describe the 
way you should think about this.

This is just an example of the fact that ordering is transitive.  So if:

	write A is ordered before write B (by an intervening UNLOCK-LOCK 
	pair), and

	write B is ordered before read C (because C reads from B and is
	on a different CPU), and

	read C is ordered before read D (by an address dependency), and

	read D is ordered before read E (because D happens to be a
	load-acquire)...

then of course A is ordered before E.

The exact nature of the individual orderings -- such as the fact that 
one of them is a dependency -- doesn't matter.  What matters is that in 
terms of the times when these operations execute, we have A < B < C < D 
< E.  Naturally this implies A < E.  As well as B < E, C < E, and so on.

As such, I don't think it's important to send much time on this in the 
documentation.  It should be pretty obvious.

The one thing to watch out for is the imprecision of the phrase "ordered 
before".  When writes are involved, the order in which the writes occur 
is not the only factor; you also have to consider the order in which 
those writes become visible to other CPUs.  In the example above, we 
know that the UNLOCK-LOCK pair causes write A to become visible to C's 
CPU before write B does.  Therefore, since write B definitely is visible 
to read C, it follows that any read on C's CPU that is ordered after C 
will observe write A.

Alan Stern

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

[Frederic Weisbecker]

Le Fri, Sep 11, 2026 at 03:05:47PM -0400, Alan Stern a écrit :
> On Fri, Sep 11, 2026 at 02:27:56PM +0200, Frederic Weisbecker wrote:
> > What I would love to see documented for example is our case: acquire semantics,
> > which are described to apply one-way from a single memory target, are also
> > transferrable to other memory targets when there is a data dependency
> > involved between them.
> 
> "Transferrable" is not the word I would use, nor does it describe the 
> way you should think about this.
> 
> This is just an example of the fact that ordering is transitive.  So if:
> 
> 	write A is ordered before write B (by an intervening UNLOCK-LOCK 
> 	pair), and
> 
> 	write B is ordered before read C (because C reads from B and is
> 	on a different CPU), and
> 
> 	read C is ordered before read D (by an address dependency), and
> 
> 	read D is ordered before read E (because D happens to be a
> 	load-acquire)...
> 
> then of course A is ordered before E.

Right, transitive fits much better here.

> 
> The exact nature of the individual orderings -- such as the fact that 
> one of them is a dependency -- doesn't matter.  What matters is that in 
> terms of the times when these operations execute, we have A < B < C < D 
> < E.  Naturally this implies A < E.  As well as B < E, C < E, and so on.
> 
> As such, I don't think it's important to send much time on this in the 
> documentation.  It should be pretty obvious.

Ok

> The one thing to watch out for is the imprecision of the phrase "ordered 
> before".  When writes are involved, the order in which the writes occur 
> is not the only factor; you also have to consider the order in which 
> those writes become visible to other CPUs.  In the example above, we 
> know that the UNLOCK-LOCK pair causes write A to become visible to C's 
> CPU before write B does.  Therefore, since write B definitely is visible 
> to read C, it follows that any read on C's CPU that is ordered after C 
> will observe write A.

Right, heh, terminology indeed matters here more than anywhere :-)

Thanks for all the explanations!

-- 
Frederic Weisbecker
SUSE Labs

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

[Frederic Weisbecker]

Le Wed, Sep 09, 2026 at 11:08:31AM +0200, Thomas Gleixner a écrit :
> 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.
> 
>  do_exit()
>    exit_signals()
>      LOCK siglock
>      tsk->flags |= PF_EXITING
>      UNLOCK siglock
> 
> So after this point anything which looks at tsk->flags under siglock
> will observe PF_EXITING and not touch the sigqueue. Nothing to see here.
> 
> > 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.
> 
> 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.
> 
> 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
> 
> H                                                             READ PID
> 							      ....
> 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.
> 
> 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.
> 
> #G can be come visible immediately but is only guaranteed to be visible
>    globally at the RELEASE of tasklist lock.
> 
> #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.
> 
> #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.
> 
> #C cannot be reordered against #B and #D
> 
> #A is therefore guaranteed to be globally visible _before_ new leader
>    observes exit_state != 0 in #F under task list lock
> 
> #G cannot be reordered against #F and obviously not against #E either.
> 
>    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.
> 
> The important part is that the visibility of #A (sigqueue stores) and #G
> (PID store) is fully ordered through task list lock.
> 
> So #H _cannot_ observe #G without observing #A - not even on PowerPC or
> similar insanities.
> 
> No?

I've always been told that ordering only works if paired.
But in practice I must confess I don't know much about hardware details.

> 
> Also doing the PID swap under sighand lock is not solving anything
> either because posix_timer_send_sigqueue() does the lookup without the
> lock simply because it does not know which task it is upfront. So it
> would have to redo and validate the lookup with the lock held.

It does the lookup without the lock but if pid is rewritten inside
the siglock on the write side and we observe the new pid from read side, then
acquiring the lock afterwards on the read side also acquires what it has
released previously (that is, everything that was acquired by tasklist_lock,
including the list_del_init()).

Not sure if my words are clear but tools/memory-model/litmus-tests/MP+polocks.litmus
explains that better.

Thanks.

-- 
Frederic Weisbecker
SUSE Labs

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

[Frederic Weisbecker]

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

[patch V2 2/8] exec: Cleanup POSIX timers right after de_thread()

[Thomas Gleixner]

From: Hyunwoo Kim <imv4bel@gmail.com>

A per-thread CPU timer holds a reference to the PID of the thread it is
attached to and, while it is armed, its node is queued in that thread's
posix_cputimers. The task is looked up by that PID.

When a non-leader thread exec()s, de_thread() changes which task owns
that PID. pid_task(timer->it.cpu.pid, PIDTYPE_PID) then returns NULL,
but the node is still queued on tsk, which is alive. timer_lock_sighand()
takes a failed lookup to mean that the node is already dequeued, so it
has nothing to undo.

begin_new_exec() calls posix_cpu_timers_exit(me) right after
exec_task_namespaces() and that removes the leftover node, so the state
normally stays invisible. But bprm->point_of_no_return is set before
de_thread(), so if unshare_files(), set_mm_exe_file(), exec_mmap() or
exec_task_namespaces() fails, the task dies before it gets there.
exit_itimers() then frees the k_itimer while its node is still queued,
and reaping tsk later erases that freed node from the rbtree.

In short:

      the non-leader thread B           the parent

  timer_create(CLOCK_THREAD_CPUTIME_ID)
  timer_settime()
    arm_timer()            // the node is queued on B
  execve()
    de_thread(B)
      exchange_tids(B, leader)  // B's PID now belongs to the leader
      release_task(leader)
        __exit_signal(leader)
          posix_cpu_timers_exit(leader)  // cleans leader's queue, not B's
          __unhash_process(leader)  // that PID has no task anymore
    exec_mmap()
      mmap_read_lock_killable(old_mm)
                                kill(B, SIGKILL)
      // -EINTR
  get_signal()
    do_exit()
      exit_itimers()
        posix_timer_delete()
          posix_cpu_timer_del()
        posix_timer_unhash_and_free()  // freed while still queued
                                wait4()
                                  release_task(B)
                                    posix_cpu_timers_exit(B)
                                      cleanup_timerqueue()
                                        timerqueue_del()  // use-after-free

Move the POSIX timer cleanup right after de_thread() before any of the
later failure conditions brings the task into do_exit().

[ tglx: Move the cleanup right after de_thread() ]

Fixes: 55e8c8eb2c7b ("posix-cpu-timers: Store a reference to a pid not a task")
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/ao7Q8miiuLAPVnWv@v4bel
---
Changes in v3:
  Move the cleanup right after de_thread() - Oleg
  Rework change log

Changes in v2:
- Add the trigger sequence and the KASAN log to the commit message.
- v1: https://lore.kernel.org/all/anfgrsPlUdwBhdrp@v4bel/
---
 fs/exec.c |   29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1115,6 +1115,17 @@ static struct file *bprm_identity_file(c
 	return bprm->file;
 }
 
+static void posixtimer_exec(struct task_struct *me)
+{
+#ifdef CONFIG_POSIX_TIMERS
+	spin_lock_irq(&me->sighand->siglock);
+	posix_cpu_timers_exit(me);
+	spin_unlock_irq(&me->sighand->siglock);
+	exit_itimers(me);
+	flush_itimer_signals();
+#endif
+}
+
 /*
  * Calling this is the point of no return. None of the failures will be
  * seen by userspace since either the process is already taking a fatal
@@ -1152,6 +1163,16 @@ int begin_new_exec(struct linux_binprm *
 	retval = de_thread(me);
 	if (retval)
 		goto out;
+
+	/*
+	 * This must be done here to ensure that POSIX CPU timers which were
+	 * armed on the current task are dequeued from me::posix_cputimers.
+	 * Otherwise in case of a TID switch the deletion of the related POSIX
+	 * timer would not remove an enqueued timer because the TID lookup
+	 * of the old TID fails.
+	 */
+	posixtimer_exec(me);
+
 	/* see the comment in check_unsafe_exec() */
 	current->fs->in_exec = 0;
 	/*
@@ -1192,14 +1213,6 @@ int begin_new_exec(struct linux_binprm *
 	if (retval)
 		goto out_unlock;
 
-#ifdef CONFIG_POSIX_TIMERS
-	spin_lock_irq(&me->sighand->siglock);
-	posix_cpu_timers_exit(me);
-	spin_unlock_irq(&me->sighand->siglock);
-	exit_itimers(me);
-	flush_itimer_signals();
-#endif
-
 	/*
 	 * Make the signal table private.
 	 */

Re: [patch V2 2/8] exec: Cleanup POSIX timers right after de_thread()

[Oleg Nesterov]

On 09/05, Thomas Gleixner wrote:
>
> Move the POSIX timer cleanup right after de_thread() before any of the
> later failure conditions brings the task into do_exit().

Reviewed-by: Oleg Nesterov <oleg@redhat.com>

Re: [patch V2 2/8] exec: Cleanup POSIX timers right after de_thread()

[Frederic Weisbecker]

Le Sat, Sep 05, 2026 at 08:59:06PM +0200, Thomas Gleixner a écrit :
> From: Hyunwoo Kim <imv4bel@gmail.com>
> 
> A per-thread CPU timer holds a reference to the PID of the thread it is
> attached to and, while it is armed, its node is queued in that thread's
> posix_cputimers. The task is looked up by that PID.
> 
> When a non-leader thread exec()s, de_thread() changes which task owns
> that PID. pid_task(timer->it.cpu.pid, PIDTYPE_PID) then returns NULL,
> but the node is still queued on tsk, which is alive. timer_lock_sighand()
> takes a failed lookup to mean that the node is already dequeued, so it
> has nothing to undo.
> 
> begin_new_exec() calls posix_cpu_timers_exit(me) right after
> exec_task_namespaces() and that removes the leftover node, so the state
> normally stays invisible. But bprm->point_of_no_return is set before
> de_thread(), so if unshare_files(), set_mm_exe_file(), exec_mmap() or
> exec_task_namespaces() fails, the task dies before it gets there.
> exit_itimers() then frees the k_itimer while its node is still queued,
> and reaping tsk later erases that freed node from the rbtree.
> 
> In short:
> 
>       the non-leader thread B           the parent
> 
>   timer_create(CLOCK_THREAD_CPUTIME_ID)
>   timer_settime()
>     arm_timer()            // the node is queued on B
>   execve()
>     de_thread(B)
>       exchange_tids(B, leader)  // B's PID now belongs to the leader
>       release_task(leader)
>         __exit_signal(leader)
>           posix_cpu_timers_exit(leader)  // cleans leader's queue, not B's
>           __unhash_process(leader)  // that PID has no task anymore
>     exec_mmap()
>       mmap_read_lock_killable(old_mm)
>                                 kill(B, SIGKILL)
>       // -EINTR
>   get_signal()
>     do_exit()
>       exit_itimers()
>         posix_timer_delete()
>           posix_cpu_timer_del()
>         posix_timer_unhash_and_free()  // freed while still queued
>                                 wait4()
>                                   release_task(B)
>                                     posix_cpu_timers_exit(B)
>                                       cleanup_timerqueue()
>                                         timerqueue_del()  // use-after-free
> 
> Move the POSIX timer cleanup right after de_thread() before any of the
> later failure conditions brings the task into do_exit().
> 
> [ tglx: Move the cleanup right after de_thread() ]
> 
> Fixes: 55e8c8eb2c7b ("posix-cpu-timers: Store a reference to a pid not a task")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Cc: stable@vger.kernel.org
> Link: https://patch.msgid.link/ao7Q8miiuLAPVnWv@v4bel

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>