The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Bradley Morgan <include@grrlz.net>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO
Date: Sun, 28 Jun 2026 19:51:58 +0200	[thread overview]
Message-ID: <akFfPk45Bkg22OW9@redhat.com> (raw)
In-Reply-To: <20260626093603.7eb06a58684e0d02f3b9d0f8@linux-foundation.org>

On 06/26, Andrew Morton wrote:
>
> On Fri, 26 Jun 2026 17:33:08 +0200 Oleg Nesterov <oleg@redhat.com> wrote:
>
> > prepare_kill_siginfo(PIDTYPE_TGID) fills si_code = SI_USER and sets
> > si_pid/si_uid in the sender's namespace. Then send_signal_locked()
> > translates si_pid/si_uid to the target's namespace.
> >
> > SEND_SIG_NOINFO produces the same result: si_code = SI_USER, and
> > __send_signal_locked() computes si_pid/si_uid directly in the target's
> > namespace. The force computation is also the same: both check if the
> > sender is visible in the target's pid namespace.
>
> The above paragraphs contain no description of any flaw.  What's wrong
> here?

Ah, sorry. nothing wrong if we forget about the fix from Bradley, but we
need this fix with or without this cleanup...

> > Note: this also fixes the kill(-1, sig) case where send_signal_locked()
> > rewrites si_pid/si_uid in the shared siginfo, corrupting it for subsequent
> > recipients. But for other group senders like __kill_pgrp_info() we still
> > need the fix from Bradley Morgan [1] who found this problem.
>
> "also fixes".  Again, what was the first fix?

Agreed. This is confusing.

> Thanks, I'll queue this for testing.

Thanks,

> Please send along some changelog
> edits sometime?

Please see the new changelog below. Does it look more clear?

Bradley, do you agree?

Oleg.
---

signal: change sys_kill() to use SEND_SIG_NOINFO

prepare_kill_siginfo(PIDTYPE_TGID) fills si_code = SI_USER and sets
si_pid/si_uid in the sender's namespace. Then send_signal_locked()
translates si_pid/si_uid to the target's namespace.

SEND_SIG_NOINFO exists precisely for the case when si_code == SI_USER
and si_pid/si_uid are the sender's ids; this is exactly what sys_kill()
does via prepare_kill_siginfo(PIDTYPE_TGID). Change sys_kill() to use
it directly.

SEND_SIG_NOINFO produces the same result: si_code = SI_USER, and
__send_signal_locked() computes si_pid/si_uid directly in the target's
namespace. The force computation is also the same: both check if the
sender is visible in the target's pid namespace.

This is just a cleanup and microoptimization (especially with [1]), this
skips the has_si_pid_and_uid() block in send_signal_locked() and offloads
the namespace translation logic to __send_signal_locked(SEND_SIG_NOINFO)
which uses the simpler computations.

NOTE: As a "side effect" this also fixes the kill(pid < 0, sig) case where
send_signal_locked() rewrites si_pid/si_uid in the shared siginfo,
corrupting it for subsequent recipients. But for other group senders like
__kill_pgrp_info() we still need the fix from Bradley Morgan [1] who found
this problem.

TODO: kill prepare_kill_siginfo() and change other users to use
SEND_SIG_NOINFO too. This needs trivial changes in __send_signal_locked()
and TP_STORE_SIGINFO().

Link: https://lore.kernel.org/all/20260622164029.11474-1-include@grrlz.net/ [1]
Signed-off-by: Oleg Nesterov <oleg@redhat.com>


  parent reply	other threads:[~2026-06-28 17:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-26 15:33 [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO Oleg Nesterov
2026-06-26 16:36 ` Andrew Morton
2026-06-26 16:49   ` Bradley Morgan
2026-06-28 17:51   ` Oleg Nesterov [this message]
2026-06-28 18:01     ` Bradley Morgan
2026-06-28 18:19       ` Oleg Nesterov
2026-06-28 19:17       ` Andrew Morton

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=akFfPk45Bkg22OW9@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=include@grrlz.net \
    --cc=linux-kernel@vger.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