posix-timers: Target group sigqueue to current task only if not exiting
PATCH 1/1 posix-timers: Target group sigqueue to current task only if not exiting
A sigqueue belonging to a posix timer, which target is not a specific thread but a whole thread group, is preferrably targeted to the current task if it is part of that thread group.
However nothing prevents a posix timer event from queueing such a sigqueue from a reaped yet running task. The interruptible code space between exit_notify() and the final call to schedule() is enough for posix_timer_fn() hrtimer to fire.
If that happens while the current task is part of the thread group target, it is proposed to handle it but since its sighand pointer may have been cleared already, the sigqueue is dropped even if there are other tasks running within the group that could handle it.
As a result posix timers with thread group wide target may miss signals when some of their threads are exiting.
Fix this with verifying that the current task hasn't been through exit_notify() before proposing it as a preferred target so as to ensure that its sighand is still here and stable.
complete_signal() might still reconsider the choice and find a better target within the group if current has passed retarget_shared_pending() already.
--- kernel/signal.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
kernel/signal.c +4 -3
@@ -1958,14 +1958,15 @@ static void posixtimer_queue_sigqueue(struct sigqueue *q, struct task_struct *t,* * Where type is not PIDTYPE_PID, signals must be delivered to the * process. In this case, prefer to deliver to current if it is in -* the same thread group as the target process, which avoids -* unnecessarily waking up a potentially idle task. +* the same thread group as the target process and its sighand is +* stable, which avoids unnecessarily waking up a potentially idle task. */ static inline struct task_struct *posixtimer_get_target (struct k_itimer *tmr) {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)) +if (t && tmr->it_pid_type !=PIDTYPE_PID && +same_thread_group (t, current) && !current->exit_state) t = current;return t; }
Signature · 1 line
2.46.0
Quoted 8 lines · Frederic Weisbecker
- * the same thread group as the target process, which avoids
- * unnecessarily waking up a potentially idle task.
+ * the same thread group as the target process and its sighand is
+ * stable, which avoids unnecessarily waking up a potentially idle task. */
static inline structtask_struct*posixtimer_get_target(structk_itimer*tmr) {
kernel/signal.c static void posixtimer_queue_sigqueue(struct sigqueue *q, struct task_struct *t, · line 1970
Quoted 6 lines
kernel/signal.c +2
-1struct 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)) +if (t && tmr->it_pid_type !=PIDTYPE_PID && +same_thread_group (t, current) && !current->exit_state) t = current;
Thanks!
Quoted 12 lines · Oleg Nesterov
Quoted 10 lines · Frederic Weisbecker
- * the same thread group as the target process, which avoids
- * unnecessarily waking up a potentially idle task.
+ * the same thread group as the target process and its sighand is
+ * stable, which avoids unnecessarily waking up a potentially idle task. */
static inline structtask_struct*posixtimer_get_target(structk_itimer*tmr){ struct task_struct *t = pid_task(tmr->it_pid, tmr->it_pid_type);
Quoted 8 lines
kernel/signal.c static void posixtimer_queue_sigqueue(struct sigqueue *q, struct task_struct *t, · line 1970
Quoted 4 lines
kernel/signal.c +2
-1-if (t && tmr->it_pid_type !=PIDTYPE_PID &&same_thread_group (t, current)) +if (t && tmr->it_pid_type !=PIDTYPE_PID && +same_thread_group (t, current) && !current->exit_state) t = current;Thanks!
Acked-by Oleg Nesterov <[email protected]>
Can't the group leader be exiting as well? Though, that's still an improvements. People usually don't do that (exiting from main w/o killing the process). So thanks for the fix.
Quoted 19 lines · Dmitry Vyukov
Quoted 17 lines · Oleg Nesterov
Quoted 14 lines · Frederic Weisbecker
- * the same thread group as the target process, which avoids
- * unnecessarily waking up a potentially idle task.
+ * the same thread group as the target process and its sighand is
+ * stable, which avoids unnecessarily waking up a potentially idle task. */
static inline structtask_struct*posixtimer_get_target(structk_itimer*tmr){ struct task_struct *t = pid_task(tmr->it_pid, tmr->it_pid_type);kernel/signal.c +2
-1-if (t && tmr->it_pid_type !=PIDTYPE_PID &&same_thread_group (t, current)) +if (t && tmr->it_pid_type !=PIDTYPE_PID && +same_thread_group (t, current) && !current->exit_state) t = current;
Quoted 5 lines
Quoted 3 lines
Thanks!
Acked-by Oleg Nesterov <[email protected]>Can't the group leader be exiting as well?
It can. It is even possible that the group leader is already a zombie.
But this is fine. release_task(zombie-or-exiting-leader) (which does __exit_signal() and clears ->sighand) won't be called until all the sub-threads have exited.
And. If all the sub-threads (and the group leader) have exited, then send_sigqueue() makes no sense, the whole process is dead. so we do not care if lock_task_sighand() fails in this case.
Quoted 3 lines
Though, that's still an
improvements. People usually don't do that (exiting from main w/o killing the process).
See above. Nothing to improve, AFAICS.
Quoted 1 line
So thanks for the fix.
Yes, thank you Anthony and Frederic!
Oleg.
Quoted 27 lines · Oleg Nesterov
Quoted 19 lines · Dmitry Vyukov
Quoted 17 lines · Oleg Nesterov
Quoted 14 lines · Frederic Weisbecker
- * the same thread group as the target process, which avoids
- * unnecessarily waking up a potentially idle task.
+ * the same thread group as the target process and its sighand is
+ * stable, which avoids unnecessarily waking up a potentially idle task. */
static inline structtask_struct*posixtimer_get_target(structk_itimer*tmr){ struct task_struct *t = pid_task(tmr->it_pid, tmr->it_pid_type);kernel/signal.c +2
-1-if (t && tmr->it_pid_type !=PIDTYPE_PID &&same_thread_group (t, current)) +if (t && tmr->it_pid_type !=PIDTYPE_PID && +same_thread_group (t, current) && !current->exit_state) t = current;Quoted 5 lines
Quoted 3 lines
Thanks!
Acked-by Oleg Nesterov <[email protected]>Can't the group leader be exiting as well?
Quoted 8 lines
It can. It is even possible that the group leader is already a zombie.
But this is fine.
release_task(zombie-or-exiting-leader)(which does__exit_signal()and clears ->sighand) won't be called until all the sub-threads have exited.And. If all the sub-threads (and the group leader) have exited, then
send_sigqueue()makes no sense, the whole process is dead. so we do not care iflock_task_sighand()fails in this case.
Ah, I see. Thanks.
Quoted 12 lines
Quoted 3 lines
Though, that's still an
improvements. People usually don't do that (exiting from main w/o killing the process).See above. Nothing to improve, AFAICS.
Quoted 1 line
So thanks for the fix.
Yes, thank you Anthony and Frederic!
Oleg.
The following commit has been merged into the timers/urgent branch of tip:
The following commit has been merged into the timers/urgent branch of tip:
Commit-ID: 63dffecfba3eddcf67a8f76d80e0c141f93d44a5 Gitweb: https://git.kernel.org/tip/63dffecfba3eddcf67a8f76d80e0c141f93d44a5 Author: Frederic Weisbecker <[email protected]> AuthorDate: Sat, 23 Nov 2024 00:48:11 +01:00 Committer: Thomas Gleixner <[email protected]> CommitterDate: Fri, 29 Nov 2024 13:19:09 +01:00
posix-timers: Target group sigqueue to current task only if not exiting
A sigqueue belonging to a posix timer, which target is not a specific thread but a whole thread group, is preferrably targeted to the current task if it is part of that thread group.
However nothing prevents a posix timer event from queueing such a sigqueue from a reaped yet running task. The interruptible code space between exit_notify() and the final call to schedule() is enough for posix_timer_fn() hrtimer to fire.
If that happens while the current task is part of the thread group target, it is proposed to handle it but since its sighand pointer may have been cleared already, the sigqueue is dropped even if there are other tasks running within the group that could handle it.
As a result posix timers with thread group wide target may miss signals when some of their threads are exiting.
Fix this with verifying that the current task hasn't been through exit_notify() before proposing it as a preferred target so as to ensure that its sighand is still here and stable.
complete_signal() might still reconsider the choice and find a better target within the group if current has passed retarget_shared_pending() already.
--- kernel/signal.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
kernel/signal.c +4 -3
@@ -1959,14 +1959,15 @@ static void posixtimer_queue_sigqueue(struct sigqueue *q, struct task_struct *t,* * Where type is not PIDTYPE_PID, signals must be delivered to the * process. In this case, prefer to deliver to current if it is in -* the same thread group as the target process, which avoids -* unnecessarily waking up a potentially idle task. +* the same thread group as the target process and its sighand is +* stable, which avoids unnecessarily waking up a potentially idle task. */ static inline struct task_struct *posixtimer_get_target (struct k_itimer *tmr) {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)) +if (t && tmr->it_pid_type !=PIDTYPE_PID && +same_thread_group (t, current) && !current->exit_state) t = current;return t; }