linux-kernel - Re: [PATCH 01/17] signal: Make SIGKILL during coredumps an explicit special case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ed8qb6mv.fsf@email.froward.int.ebiederm.org>
Date: Fri, 21 Jun 2024 11:30:32 -0500
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,  Tejun Heo <tj@...nel.org>,
  linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/17] signal: Make SIGKILL during coredumps an explicit
 special case

Oleg Nesterov <oleg@...hat.com> writes:

> Another case when I can hardly understand your reply...
>
> This patch adds a minor user visible change, that was my point.
>
> If you say that the new behaviour is better / more consistent -
> I won't really argue, "I expect no one cares" below is probably
> true. In my opinion group_exit_code = SIGKILL makes more sense
> in this special case, but again, I won't insist.
>
> But then this change should be mentioned and explained in the
> changelog, agree?

I very much agree.  It was an oversight and bug not to have included
that in the change description.

> As for "zap_threads that tests if SIGNAL_GROUP_EXIT is already set",
> this is another thing but probably I misundertood you. It is not that
> zap_threads/zap_process do not set ->group_exit_code in this case,
> in this case do_coredump() will be aborted.
>
> And to remind, zap_threads() used to set SIGNAL_GROUP_COREDUMP, not
> SIGNAL_GROUP_EXIT. Because to me the coredumping process is not exiting
> yet, it tries to handle the coredumping signal. That is why I prefer
> group_exit_code = SIGKILL if it is killed during the dump. But this is
> slightly offtopic today.

Slightly.

A major goal of this set of changes is to unify all of the process
teardown in complete_signal, do_group_exit, and zap_process into a
single subroutine for consistency.

When a coredump is not generated the code for dumpable signals and
other fatal signals should be the same.  Including short circuit
delivery.  It isn't today.

My rougher in progress patchset that follows this one makes, teaches
get_signal to dequeue signals that have been processed with short
circuit delivery and makes it so that do_coredump is just a little
bit of extra code that runs.  With the net result that all of the code
is simpler and easier to reason about.

Messing with the coredump code today is a real pain because of io_uring
and those funny interactions.

Eric