Skip to contentSkip to conversation

FAILED: patch "[PATCH] signal: Prevent exec() race" failed to apply to 6.18-stable tree

PATCH New [email protected] 2 participants 3 messages Sep 22 Stable branch

lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 6.18-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.18.y
git checkout FETCH_HEAD
git cherry-pick -x d2710c8d938ae6a825a6463158e6e6f31eac792a
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026092201-refining-banner-c772@gregkh' --subject-prefix 'PATCH 6.18.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From d2710c8d938ae6a825a6463158e6e6f31eac792a Mon Sep 17 00:00:00 2001
From: Thomas Gleixner <[email protected]>
Date: Fri, 11 Sep 2026 11:09:12 +0200
Subject: [PATCH] signal: Prevent exec() race

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.

There has been quite some discussion about the lockless flush and the non-leader exec case on weakly ordered systems. The problem is that a third party which tries to send a posix timer signal relies on the PID lookup to find the target task and that lookup might result in the new leader when the signal was originaly directed to the old leader. In case that the signal was queued on the old leader then the lockless flush raised a concern over the following situation:

old_leader new_leader third party

A: flush_list() // list_del_init() stores to sigqueue

LOCK (tasklist)
old_leader->exit_state = EXIT_ZOMBIE;
B: UNLOCK (tasklist)

C: LOCK (tasklist)

			if (old_leader->exit_state)
			   transfer_tids()
D:			     store PID
						posix_timer_send_sigqueue()
						// Observes #D so t = new_leader

E: t = get_target()

F: LOCK (sighand)

G: if (list_empty(sigqueue))
list_add(sigqueue)

The concern was that the third party might observe #D but not observe #A and therefore would proceed to #G while the list_del() stores (#A) in flush_list() are not visible yet, which could result in list corruption.

That would be possible if looking at it solely from a RELEASE+ACQUIRE ordering point of view, but B-C is a UNLOCK+LOCK hand-over, which is not the same as RELEASE+ACQUIRE:

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

As B-C is UNLOCK+LOCK, which is RCtso and that does impose store order, A stores must happen before the D store.

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

kernel/exit.c +6 -5
index 4e028f157597..424c44a42a4d 100644
@@ -302,12 +302,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);
 
kernel/signal.c +86 -31

188 lines not on this page · raw message

maintainer
#1 lore raw

PATCH 1/2 signal: Move MMCID exit out of sighand lock

From: Thomas Gleixner <[email protected]>

[ Upstream commit 2b1642b881088bbf73fcb1147c474a198ec46729 ]

There is no need anymore to keep this under sighand lock as the current code and the upcoming replacement are not depending on the exit state of a task anymore.

That allows to use a mutex in the exit path.

[Backport to 6.18: Keep the MMCID declarations and CONFIG_SCHED_MM_CID stub in include/linux/mm.h, where they live in this tree, and rename the existing exit helper and its barrier-comment references there and in kernel/sched/core.c. Preserve the stable helper's runqueue locking, mm_cid_active store, memory barrier and per-CPU CID release, as well as the separate sched_mm_cid_before_execve() implementation. This tree does not have the upstream MMCID refactoring that moved the declarations to sched.h and made the exec helper call the exit helper.

Move the existing exit cleanup before exit_signals() in do_exit(), as upstream does. This removes the MMCID calls from the exit_signals() code rewritten by d2710c8d938ae ("signal: Prevent exec() race"), allowing that fix to apply without importing unrelated MMCID changes. No functions are added.]

[ sashal: Reduced backport -- upstream 2b1642b881088 touches 4 file(s), this
backport carries 4. Not backported here:
include/linux/sched.h
This note is generated from the file lists only; see the resolution record for the reasoning. ]

Stable-dep-of: d2710c8d938a ("signal: Prevent exec() race")

---
 include/linux/mm.h  | 4 ++--
 kernel/exit.c       | 1 +
 kernel/sched/core.c | 6 +++---
 kernel/signal.c     | 2 --
 4 files changed, 6 insertions(+), 7 deletions(-)
include/linux/mm.h +2 -2
index 50710a5d4a05f..2c0503bbcc190 100644
@@ -2516,7 +2516,7 @@ struct zap_details {
 void sched_mm_cid_before_execve(struct task_struct *t);
 void sched_mm_cid_after_execve(struct task_struct *t);
 void sched_mm_cid_fork(struct task_struct *t);
-void sched_mm_cid_exit_signals(struct task_struct *t);
+void sched_mm_cid_exit(struct task_struct *t);
 static inline int task_mm_cid(struct task_struct *t)
 {
 	return t->mm_cid;
@@ -2525,7 +2525,7 @@ static inline int task_mm_cid(struct task_struct *t)
 static inline void sched_mm_cid_before_execve(struct task_struct *t) { }
 static inline void sched_mm_cid_after_execve(struct task_struct *t) { }
 static inline void sched_mm_cid_fork(struct task_struct *t) { }
-static inline void sched_mm_cid_exit_signals(struct task_struct *t) { }
+static inline void sched_mm_cid_exit(struct task_struct *t) { }
 static inline int task_mm_cid(struct task_struct *t)
 {
 	/*
kernel/exit.c +1 -0
index d60c1241923a9..de904800e630a 100644
@@ -924,6 +924,7 @@ void __noreturn do_exit(long code)
 	user_events_exit(tsk);
 
 	io_uring_files_cancel();
+	sched_mm_cid_exit(tsk);
 	exit_signals(tsk);  /* sets PF_EXITING */
 
 	seccomp_filter_release(tsk);
kernel/sched/core.c +3 -3
index 43f3ac16b0e8a..aad9813bd6171 100644
@@ -10545,7 +10545,7 @@ int __sched_mm_cid_migrate_from_try_steal_cid(struct rq *src_rq,
 	 *
 	 * The implicit barrier after cmpxchg per-mm/cpu cid before loading
 	 * rq->curr->mm_cid_active matches the barrier in
-	 * sched_mm_cid_exit_signals(), sched_mm_cid_before_execve(), and
+	 * sched_mm_cid_exit(), sched_mm_cid_before_execve(), and
 	 * sched_mm_cid_after_execve() between store to t->mm_cid_active and
 	 * load of per-mm/cpu cid.
 	 */
@@ -10665,7 +10665,7 @@ static void sched_mm_cid_remote_clear(struct mm_struct *mm, struct mm_cid *pcpu_
 	 *
 	 * The implicit barrier after cmpxchg per-mm/cpu cid before loading
 	 * rq->curr->mm_cid_active matches the barrier in
-	 * sched_mm_cid_exit_signals(), sched_mm_cid_before_execve(), and
+	 * sched_mm_cid_exit(), sched_mm_cid_before_execve(), and
 	 * sched_mm_cid_after_execve() between store to t->mm_cid_active and
 	 * load of per-mm/cpu cid.
 	 */
@@ -10810,7 +10810,7 @@ void task_tick_mm_cid(struct rq *rq, struct task_struct *curr)
 	task_work_add(curr, work, TWA_RESUME);
 }
 
-void sched_mm_cid_exit_signals(struct task_struct *t)
+void sched_mm_cid_exit(struct task_struct *t)
 {
 	struct mm_struct *mm = t->mm;
 	struct rq *rq;
kernel/signal.c +0 -2
index b18f07a5b62d6..ec64845778457 100644
@@ -3150,7 +3150,6 @@ void exit_signals(struct task_struct *tsk)
 	cgroup_threadgroup_change_begin(tsk);
 
 	if (thread_group_empty(tsk) || (tsk->signal->flags & SIGNAL_GROUP_EXIT)) {
-		sched_mm_cid_exit_signals(tsk);
 		tsk->flags |= PF_EXITING;
 		cgroup_threadgroup_change_end(tsk);
 		return;
@@ -3161,7 +3160,6 @@ void exit_signals(struct task_struct *tsk)
 	 * From now this task is not visible for group-wide signals,
 	 * see wants_signal(), do_signal_stop().
 	 */
-	sched_mm_cid_exit_signals(tsk);
 	tsk->flags |= PF_EXITING;
 
 	cgroup_threadgroup_change_end(tsk);
Signature · 1 line
2.53.0
maintainer
#2 lore raw

PATCH 2/2 signal: Prevent exec() race

From: Thomas Gleixner <[email protected]>

[ Upstream commit d2710c8d938ae6a825a6463158e6e6f31eac792a ]

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.

There has been quite some discussion about the lockless flush and the non-leader exec case on weakly ordered systems. The problem is that a third party which tries to send a posix timer signal relies on the PID lookup to find the target task and that lookup might result in the new leader when the signal was originaly directed to the old leader. In case that the signal was queued on the old leader then the lockless flush raised a concern over the following situation:

old_leader new_leader third party

A: flush_list() // list_del_init() stores to sigqueue

LOCK (tasklist)
old_leader->exit_state = EXIT_ZOMBIE;
B: UNLOCK (tasklist)

C: LOCK (tasklist)

			if (old_leader->exit_state)
			   transfer_tids()
D:			     store PID
						posix_timer_send_sigqueue()
						// Observes #D so t = new_leader

E: t = get_target()

F: LOCK (sighand)

G: if (list_empty(sigqueue))
list_add(sigqueue)

The concern was that the third party might observe #D but not observe #A and therefore would proceed to #G while the list_del() stores (#A) in flush_list() are not visible yet, which could result in list corruption.

That would be possible if looking at it solely from a RELEASE+ACQUIRE ordering point of view, but B-C is a UNLOCK+LOCK hand-over, which is not the same as RELEASE+ACQUIRE:

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

As B-C is UNLOCK+LOCK, which is RCtso and that does impose store order, A stores must happen before the D store.

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

---
 kernel/exit.c   |  11 ++---
 kernel/signal.c | 115 +++++++++++++++++++++++++++++++++++-------------
 2 files changed, 91 insertions(+), 35 deletions(-)
kernel/exit.c +6 -5
index de904800e630a..e38b2bb850978 100644
@@ -304,12 +304,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);
 
kernel/signal.c +85 -30

187 lines not on this page · raw message

Signature · 1 line
2.53.0