mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: Thomas Gleixner <tglx@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>,
	 Frederic Weisbecker <frederic@kernel.org>,
	 Hyunwoo Kim <imv4bel@gmail.com>,
	brauner@kernel.org,  peterz@infradead.org,
	 anna-maria@linutronix.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] signal: Use list_del_init_careful() in flush_sigqueue()
Date: Thu, 27 Aug 2026 13:43:57 -0500	[thread overview]
Message-ID: <87mru7h09e.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <87se3zgb3m.ffs@fw13> (Thomas Gleixner's message of "Thu, 27 Aug 2026 11:35:09 +0200")

Thomas Gleixner <tglx@kernel.org> writes:

> On Wed, Aug 26 2026 at 22:29, Eric W. Biederman wrote:
>> Could the posix timers cleanup be moved from __exit_signal in
>> release_task (which is really for cleanup for zombies but has
>> been historically abused because it was the only place that
>> knew when the whole group was dead), into somewhere in do_exit?
>>
>> Say near where hrtimers_cancel and exit_itimers are called.
>
> That's only for the group_dead case in do_exit().
>
> But a single task existing from a process needs to clean up
> task::pending, i.e. signals which are targeted at the exiting task.
>
> The safe and obvious place is to do that is _after_ setting
> task::sighand to NULL because that ensures that no new signal can be
> queued and nothing can touch task::pending anymore.

Not really.  Using release_task (which is what is called when a zombie
is reaped) for anything except cleaning up state that a zombie needs is
a bit of a misfeature.  Timers should not be active in a zombie.
Signals also should be deactivated long before then.


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.

There is the goofy case that we need to be able to deliver signals
to the entire process through a zombie thread (in particular a zombie
thread group leader).  That goofy case unfortunately means that except
for signals to just the thread we have to deliver signals when
PF_EXITING is set.  That goofy case also unfortunately means that
sighand_struct needs to be retained past the point where signals
are delivered.

>> Then perhaps move the posix timer disabling before de_thread?
>
> That does not work because between that and de_thread() any thread of
> the thread group can create a new posix timer unless we prevent that
> somehow in timer_create().
>
> So in any case we need some mechanism in posixtimer related code to
> handle this situation gracefully.

Which is a completely reasonable reason to focus on that mechanism,
and leave the rest alone.


I suspect the current crop of bug finding may keep coming until all of
the weird corner cases in process cleanup, exec, and signal handling are
all sorted out.  So figuring out how to make the code make better sense
in the long run appears to be a good idea.

Eric

  reply	other threads:[~2026-08-27 19:03 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-22  5:37 Hyunwoo Kim
2026-08-22 10:27 ` Bradley Morgan
2026-08-23 12:47 ` Oleg Nesterov
2026-08-24  2:53   ` Hyunwoo Kim
2026-08-24  8:28     ` Oleg Nesterov
2026-08-24  8:04 ` Thomas Gleixner
2026-08-24  9:45   ` Thomas Gleixner
2026-08-24 11:02     ` Oleg Nesterov
2026-08-24 11:54       ` Oleg Nesterov
2026-08-24 13:59         ` Frederic Weisbecker
2026-08-24 14:29           ` Oleg Nesterov
2026-08-25 16:58           ` Thomas Gleixner
2026-08-25 18:53             ` Oleg Nesterov
2026-08-25 19:58               ` Thomas Gleixner
2026-08-26  9:36                 ` Oleg Nesterov
2026-08-26 19:19                   ` Thomas Gleixner
2026-08-26 19:32                     ` Oleg Nesterov
2026-08-27  3:29                       ` Eric W. Biederman
2026-08-27  9:35                         ` Thomas Gleixner
2026-08-27 18:43                           ` Eric W. Biederman [this message]
2026-08-27 22:56                             ` Thomas Gleixner
2026-08-27 12:24                       ` Thomas Gleixner
2026-08-27 17:51                         ` Thomas Gleixner
2026-08-24 12:11       ` Thomas Gleixner
2026-08-24 16:31     ` Frederic Weisbecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mru7h09e.fsf@email.froward.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=anna-maria@linutronix.de \
    --cc=brauner@kernel.org \
    --cc=frederic@kernel.org \
    --cc=imv4bel@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®