Re: [PATCH] signal: Use list_del_init_careful() in flush_sigqueue()

[Thomas Gleixner <tglx@kernel.org>]

On Fri, Aug 28 2026 at 00:56, Thomas Gleixner wrote:
> On Thu, Aug 27 2026 at 13:43, Eric W. Biederman wrote:
>> The obvious place to clean up task::pending i.e. signals is in
>> exit_signals().
>>
>> I expect if I read through the history again that I would find that
>> exit_signals() used to call flush_sigqueue, and that during the addition
>> of posix thread signal handling flush_sigqueue was moved into
>> __exit_signal in release_task because knowing if the entire thread group
>> is dead was not available during that part of 2.5.
>>
>> We should honor PF_EXITING on a task and simply stop delivering
>> signals to it.  Today the code goes halfway there and does not
>> set sig-pending after PF_EXITING is set.
>
> If flushing tsk::pending in exit_signals() is safe and stopping signals
> to be queued when PF_EXITING is observed under sighand lock, then sure
> that's the right thing to do. I'll look into that tomorrow.

By some definition of tomorrow. :)

Something like the below?

Thanks,

        tglx
---
--- 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
@@ -1030,6 +1030,10 @@ static int __send_signal_locked(int sig,
 	lockdep_assert_held(&t->sighand->siglock);
 
 	result = TRACE_SIGNAL_IGNORED;
+
+	if (unlikely(type == PIDTYPE_PID && (t->flags & PF_EXITING)))
+		goto ret;
+
 	if (!prepare_signal(sig, t, force))
 		goto ret;
 
@@ -1990,6 +1994,9 @@ void posixtimer_send_sigqueue(struct k_i
 	if (!likely(lock_task_sighand(t, &flags)))
 		return;
 
+	if (unlikely(tmr->it_pid_type == PIDTYPE_PID && (t->flags & PF_EXITING)))
+		return;
+
 	/*
 	 * Update @tmr::sigqueue_seq for posix timer signals with sighand
 	 * locked to prevent a race against dequeue_signal().
@@ -3118,6 +3125,16 @@ static void retarget_shared_pending(stru
 	}
 }
 
+/*
+ * tsk::flags has PF_EXITING set which prevents signals to be queued for on
+ * tsk::pending. Nothing else can touch the tsk::pending anymore so it can be
+ * flushed lockless.
+ */
+static inline void flush_pending_unlocked(struct task_struct *tsk)
+{
+	flush_sigqueue(&tsk->pending);
+}
+
 void exit_signals(struct task_struct *tsk)
 {
 	int group_stop = 0;
@@ -3130,8 +3147,10 @@ void exit_signals(struct task_struct *ts
 	cgroup_threadgroup_change_begin(tsk);
 
 	if (thread_group_empty(tsk) || (tsk->signal->flags & SIGNAL_GROUP_EXIT)) {
-		tsk->flags |= PF_EXITING;
+		scoped_guard(spinlock_irq, &tsk->sighand->siglock)
+			tsk->flags |= PF_EXITING;
 		cgroup_threadgroup_change_end(tsk);
+		flush_pending_unlocked(tsk);
 		return;
 	}
 
@@ -3157,6 +3176,8 @@ void exit_signals(struct task_struct *ts
 out:
 	spin_unlock_irq(&tsk->sighand->siglock);
 
+	flush_pending_unlocked(tsk);
+
 	/*
 	 * If group stop has completed, deliver the notification.  This
 	 * should always go to the real parent of the group leader.