Re: [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO

[Bradley Morgan]

On June 28, 2026 6:51:58 PM GMT+01:00, Oleg Nesterov <oleg@xxxxxxxxxx>
wrote:
>On 06/26, Andrew Morton wrote:
>>
>> On Fri, 26 Jun 2026 17:33:08 +0200 Oleg Nesterov <oleg@xxxxxxxxxx>
>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...

Ack.

>> > 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.

Made me a bit confused! Heh.

>> 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@xxxxxxxxx/ [1]
>Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>

Code wise, please add my tag.

Reviewed-by: Bradley Morgan <include@xxxxxxxxx>

I would do acked by personally but no idea if that's liked over in this
subsystm

Description wise, I'm sure that looks fine


Andrew, could you please take my fix?

>

Thanks!