All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO
@ 2026-06-26 15:33 Oleg Nesterov
  2026-06-26 16:36 ` Andrew Morton
  0 siblings, 1 reply; 7+ messages in thread
From: Oleg Nesterov @ 2026-06-26 15:33 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Bradley Morgan, Eric W. Biederman, linux-kernel

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.

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.

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>
---
 kernel/signal.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 077effd21582..12edbf43d678 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3966,11 +3966,7 @@ static void prepare_kill_siginfo(int sig, struct kernel_siginfo *info,
  */
 SYSCALL_DEFINE2(kill, pid_t, pid, int, sig)
 {
-	struct kernel_siginfo info;
-
-	prepare_kill_siginfo(sig, &info, PIDTYPE_TGID);
-
-	return kill_something_info(sig, &info, pid);
+	return kill_something_info(sig, SEND_SIG_NOINFO, pid);
 }
 
 /*
-- 
2.52.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO
  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
  0 siblings, 2 replies; 7+ messages in thread
From: Andrew Morton @ 2026-06-26 16:36 UTC (permalink / raw)
  To: Oleg Nesterov; +Cc: Bradley Morgan, Eric W. Biederman, linux-kernel

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?

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

> 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().
> 
> ...
>
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3966,11 +3966,7 @@ static void prepare_kill_siginfo(int sig, struct kernel_siginfo *info,
>   */
>  SYSCALL_DEFINE2(kill, pid_t, pid, int, sig)
>  {
> -	struct kernel_siginfo info;
> -
> -	prepare_kill_siginfo(sig, &info, PIDTYPE_TGID);
> -
> -	return kill_something_info(sig, &info, pid);
> +	return kill_something_info(sig, SEND_SIG_NOINFO, pid);
>  }

Thanks, I'll queue this for testing.  Please send along some changelog
edits sometime?


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO
  2026-06-26 16:36 ` Andrew Morton
@ 2026-06-26 16:49   ` Bradley Morgan
  2026-06-28 17:51   ` Oleg Nesterov
  1 sibling, 0 replies; 7+ messages in thread
From: Bradley Morgan @ 2026-06-26 16:49 UTC (permalink / raw)
  To: Andrew Morton, Oleg Nesterov; +Cc: Eric W. Biederman, linux-kernel

On June 26, 2026 5:36:03 PM GMT+01:00, Andrew Morton
<akpm@linux-foundation.org> 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?
>
>> 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?
>
>> 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().
>> 
>> ...
>>
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -3966,11 +3966,7 @@ static void prepare_kill_siginfo(int sig, struct
>kernel_siginfo *info,
>>   */
>>  SYSCALL_DEFINE2(kill, pid_t, pid, int, sig)
>>  {
>> -	struct kernel_siginfo info;
>> -
>> -	prepare_kill_siginfo(sig, &info, PIDTYPE_TGID);
>> -
>> -	return kill_something_info(sig, &info, pid);
>> +	return kill_something_info(sig, SEND_SIG_NOINFO, pid);
>>  }
>
>Thanks, I'll queue this for testing.  Please send along some changelog
>edits sometime?
>
>
Fair enough.

If you want, please add


Reviewed-by: Bradley Morgan <include@grrlz.net>

:)
Thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO
  2026-06-26 16:36 ` Andrew Morton
  2026-06-26 16:49   ` Bradley Morgan
@ 2026-06-28 17:51   ` Oleg Nesterov
  2026-06-28 18:01     ` Bradley Morgan
  1 sibling, 1 reply; 7+ messages in thread
From: Oleg Nesterov @ 2026-06-28 17:51 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Bradley Morgan, Eric W. Biederman, linux-kernel

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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO
  2026-06-28 17:51   ` Oleg Nesterov
@ 2026-06-28 18:01     ` Bradley Morgan
  2026-06-28 18:19       ` Oleg Nesterov
  2026-06-28 19:17       ` Andrew Morton
  0 siblings, 2 replies; 7+ messages in thread
From: Bradley Morgan @ 2026-06-28 18:01 UTC (permalink / raw)
  To: Oleg Nesterov, Andrew Morton; +Cc: Eric W. Biederman, linux-kernel

On June 28, 2026 6:51:58 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com>
wrote:
>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...

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@grrlz.net/ [1]
>Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Code wise, please add my tag.

Reviewed-by: Bradley Morgan <include@grrlz.net>

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!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO
  2026-06-28 18:01     ` Bradley Morgan
@ 2026-06-28 18:19       ` Oleg Nesterov
  2026-06-28 19:17       ` Andrew Morton
  1 sibling, 0 replies; 7+ messages in thread
From: Oleg Nesterov @ 2026-06-28 18:19 UTC (permalink / raw)
  To: Bradley Morgan; +Cc: Andrew Morton, Eric W. Biederman, linux-kernel

On 06/28, Bradley Morgan wrote:
>
> Code wise, please add my tag.
>
> Reviewed-by: Bradley Morgan <include@grrlz.net>

Thanks!

> Andrew, could you please take my fix?

Yes, I have already acked

	[PATCH] signal: avoid shared siginfo namespace rewrites
	https://lore.kernel.org/all/20260622164029.11474-1-include@grrlz.net/

and afaics Eric doesn't object to this fix.

This changelog adds a link to this fix. We will send more changes on
top of it.

Oleg.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] signal: change sys_kill() to use SEND_SIG_NOINFO
  2026-06-28 18:01     ` Bradley Morgan
  2026-06-28 18:19       ` Oleg Nesterov
@ 2026-06-28 19:17       ` Andrew Morton
  1 sibling, 0 replies; 7+ messages in thread
From: Andrew Morton @ 2026-06-28 19:17 UTC (permalink / raw)
  To: Bradley Morgan; +Cc: Oleg Nesterov, Eric W. Biederman, linux-kernel

On Sun, 28 Jun 2026 19:01:53 +0100 Bradley Morgan <include@grrlz.net> wrote:

> >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>
> 
> Code wise, please add my tag.
> 
> Reviewed-by: Bradley Morgan <include@grrlz.net>

Thanks.

I updated the changelog for this patch.

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

Both are liked ;)

fwiw, I consider Reviewed-by to be stronger than Acked-by.  I suspect
the latter sometimes means "cool changelog!".

> Description wise, I'm sure that looks fine
> 
> 
> Andrew, could you please take my fix?

There are many fixes, so please be specific to help avoid mistakes!

I think you're referring to
	https://lore.kernel.org/all/86a8857d58d43ee26a8b365b837fd24830343494.1782159692.git.include@grrlz.net/

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-28 19:18 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-06-28 18:01     ` Bradley Morgan
2026-06-28 18:19       ` Oleg Nesterov
2026-06-28 19:17       ` Andrew Morton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.