[PATCH] signal: avoid shared siginfo namespace rewrites

[PATCH] signal: avoid shared siginfo namespace rewrites

[Bradley Morgan]

send_signal_locked() rewrites sender ids for the target namespace.
Group sends reuse the same siginfo, so one recipient can affect the
next.

Copy the siginfo before changing it.

Fixes: 7a0cf094944e ("signal: Correct namespace fixups of si_pid and si_uid")
Cc: stable@vger.kernel.org
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
 kernel/signal.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/signal.c b/kernel/signal.c
index b9fc7be1a169..d72d9be3a992 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1181,6 +1181,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
 int send_signal_locked(int sig, struct kernel_siginfo *info,
 		       struct task_struct *t, enum pid_type type)
 {
+	struct kernel_siginfo rewritten;
 	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
 	bool force = false;
 
@@ -1194,6 +1195,9 @@ int send_signal_locked(int sig, struct kernel_siginfo *info,
 		/* SIGKILL and SIGSTOP is special or has ids */
 		struct user_namespace *t_user_ns;
 
+		rewritten = *info;
+		info = &rewritten;
+
 		rcu_read_lock();
 		t_user_ns = task_cred_xxx(t, user_ns);
 		if (current_user_ns() != t_user_ns) {
-- 
2.53.0

Re: [PATCH] signal: avoid shared siginfo namespace rewrites

[Oleg Nesterov]

On 06/22, Bradley Morgan wrote:
>
> send_signal_locked() rewrites sender ids for the target namespace.
> Group sends reuse the same siginfo, so one recipient can affect the
> next.

Hmm... I'll re-read this change tomorrow after sleep, but I am almost sure
you are you are right anyway...

I am wondering if we can conditionalize the "swap(rewritten, info)" logic
with your patch, most probably this makes no sense...

May I suggest another change on top of your fix? Make the "kernel_siginfo *info"
arg of send_signal_locked() "const". To make it more clear. Yes, the signature
of has_si_pid_and_uid() should be changed too. Up to you.

Thanks,

Oleg.

> Copy the siginfo before changing it.
>
> Fixes: 7a0cf094944e ("signal: Correct namespace fixups of si_pid and si_uid")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bradley Morgan <include@grrlz.net>
> ---
>  kernel/signal.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index b9fc7be1a169..d72d9be3a992 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1181,6 +1181,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
>  int send_signal_locked(int sig, struct kernel_siginfo *info,
>  		       struct task_struct *t, enum pid_type type)
>  {
> +	struct kernel_siginfo rewritten;
>  	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
>  	bool force = false;
> 
> @@ -1194,6 +1195,9 @@ int send_signal_locked(int sig, struct kernel_siginfo *info,
>  		/* SIGKILL and SIGSTOP is special or has ids */
>  		struct user_namespace *t_user_ns;
> 
> +		rewritten = *info;
> +		info = &rewritten;
> +
>  		rcu_read_lock();
>  		t_user_ns = task_cred_xxx(t, user_ns);
>  		if (current_user_ns() != t_user_ns) {
> --
> 2.53.0
>

Re: [PATCH] signal: avoid shared siginfo namespace rewrites

[Bradley Morgan]

On June 22, 2026 6:46:37 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com>
wrote:
>On 06/22, Bradley Morgan wrote:
>>
>> send_signal_locked() rewrites sender ids for the target namespace.
>> Group sends reuse the same siginfo, so one recipient can affect the
>> next.
>
>Hmm... I'll re-read this change tomorrow after sleep, but I am almost sure
>you are you are right anyway...

Sure! Feel free to take ur time!

>I am wondering if we can conditionalize the "swap(rewritten, info)" logic
>with your patch, most probably this makes no sense...
>
>May I suggest another change on top of your fix? Make the "kernel_siginfo
>*info"
>arg of send_signal_locked() "const". To make it more clear. Yes, the
>signature
>of has_si_pid_and_uid() should be changed too. Up to you.

I'll do it. I don't mind.

>Thanks,
>
>Oleg.
>
>> Copy the siginfo before changing it.
>>
>> Fixes: 7a0cf094944e ("signal: Correct namespace fixups of si_pid and
>si_uid")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bradley Morgan <include@grrlz.net>
>> ---
>>  kernel/signal.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index b9fc7be1a169..d72d9be3a992 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -1181,6 +1181,7 @@ static inline bool has_si_pid_and_uid(struct
>kernel_siginfo *info)
>>  int send_signal_locked(int sig, struct kernel_siginfo *info,
>>  		       struct task_struct *t, enum pid_type type)
>>  {
>> +	struct kernel_siginfo rewritten;
>>  	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
>>  	bool force = false;
>> 
>> @@ -1194,6 +1195,9 @@ int send_signal_locked(int sig, struct
>kernel_siginfo *info,
>>  		/* SIGKILL and SIGSTOP is special or has ids */
>>  		struct user_namespace *t_user_ns;
>> 
>> +		rewritten = *info;
>> +		info = &rewritten;
>> +
>>  		rcu_read_lock();
>>  		t_user_ns = task_cred_xxx(t, user_ns);
>>  		if (current_user_ns() != t_user_ns) {
>> --
>> 2.53.0
>>
>
>

Thanks!

[PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites

[Bradley Morgan]

send_signal_locked() rewrites sender ids for the target namespace.
Group sends reuse the same siginfo, so one recipient can affect the
next.

Copy the siginfo before changing it.

Fixes: 7a0cf094944e ("signal: Correct namespace fixups of si_pid and si_uid")
Cc: stable@vger.kernel.org
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
Changes since v1:
- No code changes in this patch.
- Add patch 2 for Oleg's const suggestion.
- Link to v1:
  https://lore.kernel.org/all/0873AC4A-3CB2-4F7B-BFE6-75D855AD22DC@grrlz.net/T/#m89955d13f10807c316d34cc76680d690a2d95b31

 kernel/signal.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/signal.c b/kernel/signal.c
index b9fc7be1a169..d72d9be3a992 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1181,6 +1181,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
 int send_signal_locked(int sig, struct kernel_siginfo *info,
 		       struct task_struct *t, enum pid_type type)
 {
+	struct kernel_siginfo rewritten;
 	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
 	bool force = false;
 
@@ -1194,6 +1195,9 @@ int send_signal_locked(int sig, struct kernel_siginfo *info,
 		/* SIGKILL and SIGSTOP is special or has ids */
 		struct user_namespace *t_user_ns;
 
+		rewritten = *info;
+		info = &rewritten;
+
 		rcu_read_lock();
 		t_user_ns = task_cred_xxx(t, user_ns);
 		if (current_user_ns() != t_user_ns) {
-- 
2.53.0

[PATCH v2 2/2] signal: make send_signal_locked() take const siginfo

[Bradley Morgan]

send_signal_locked() should not change the caller's siginfo. Make that
part of the type and keep the local rewrite on its copy.

Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
Changes since v1:
- New patch from Oleg's suggestion.
- Link to Oleg's suggestion:
  https://lore.kernel.org/all/0873AC4A-3CB2-4F7B-BFE6-75D855AD22DC@grrlz.net/T/#m5f8a2d54928efff41de539969b68149e1ec5fca4

 include/linux/signal.h        |  2 +-
 include/trace/events/signal.h |  4 ++--
 kernel/signal.c               | 20 +++++++++++---------
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/include/linux/signal.h b/include/linux/signal.h
index f19816832f05..a1ba8c5973c6 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -283,7 +283,7 @@ extern int do_send_sig_info(int sig, struct kernel_siginfo *info,
 				struct task_struct *p, enum pid_type type);
 extern int group_send_sig_info(int sig, struct kernel_siginfo *info,
 			       struct task_struct *p, enum pid_type type);
-extern int send_signal_locked(int sig, struct kernel_siginfo *info,
+extern int send_signal_locked(int sig, const struct kernel_siginfo *info,
 			      struct task_struct *p, enum pid_type type);
 extern int sigprocmask(int, sigset_t *, sigset_t *);
 extern void set_current_blocked(sigset_t *);
diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
index 1db7e4b07c01..05a46135ee34 100644
--- a/include/trace/events/signal.h
+++ b/include/trace/events/signal.h
@@ -49,8 +49,8 @@ enum {
  */
 TRACE_EVENT(signal_generate,
 
-	TP_PROTO(int sig, struct kernel_siginfo *info, struct task_struct *task,
-			int group, int result),
+	TP_PROTO(int sig, const struct kernel_siginfo *info,
+		 struct task_struct *task, int group, int result),
 
 	TP_ARGS(sig, info, task, group, result),
 
diff --git a/kernel/signal.c b/kernel/signal.c
index d72d9be3a992..26e8b8e1d03c 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1037,7 +1037,7 @@ static inline bool legacy_queue(struct sigpending *signals, int sig)
 	return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
 }
 
-static int __send_signal_locked(int sig, struct kernel_siginfo *info,
+static int __send_signal_locked(int sig, const struct kernel_siginfo *info,
 				struct task_struct *t, enum pid_type type, bool force)
 {
 	struct sigpending *pending;
@@ -1154,7 +1154,7 @@ static int __send_signal_locked(int sig, struct kernel_siginfo *info,
 	return ret;
 }
 
-static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
+static inline bool has_si_pid_and_uid(const struct kernel_siginfo *info)
 {
 	bool ret = false;
 	switch (siginfo_layout(info->si_signo, info->si_code)) {
@@ -1178,10 +1178,11 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
 	return ret;
 }
 
-int send_signal_locked(int sig, struct kernel_siginfo *info,
+int send_signal_locked(int sig, const struct kernel_siginfo *info,
 		       struct task_struct *t, enum pid_type type)
 {
 	struct kernel_siginfo rewritten;
+	const struct kernel_siginfo *send_info = info;
 	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
 	bool force = false;
 
@@ -1196,26 +1197,27 @@ int send_signal_locked(int sig, struct kernel_siginfo *info,
 		struct user_namespace *t_user_ns;
 
 		rewritten = *info;
-		info = &rewritten;
+		send_info = &rewritten;
 
 		rcu_read_lock();
 		t_user_ns = task_cred_xxx(t, user_ns);
 		if (current_user_ns() != t_user_ns) {
-			kuid_t uid = make_kuid(current_user_ns(), info->si_uid);
-			info->si_uid = from_kuid_munged(t_user_ns, uid);
+			kuid_t uid = make_kuid(current_user_ns(), rewritten.si_uid);
+
+			rewritten.si_uid = from_kuid_munged(t_user_ns, uid);
 		}
 		rcu_read_unlock();
 
 		/* A kernel generated signal? */
-		force = (info->si_code == SI_KERNEL);
+		force = (rewritten.si_code == SI_KERNEL);
 
 		/* From an ancestor pid namespace? */
 		if (!task_pid_nr_ns(current, task_active_pid_ns(t))) {
-			info->si_pid = 0;
+			rewritten.si_pid = 0;
 			force = true;
 		}
 	}
-	return __send_signal_locked(sig, info, t, type, force);
+	return __send_signal_locked(sig, send_info, t, type, force);
 }
 
 static void print_fatal_signal(int signr)
-- 
2.53.0

Re: [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo

[Oleg Nesterov]

On 06/22, Bradley Morgan wrote:
>
> send_signal_locked() should not change the caller's siginfo. Make that
> part of the type and keep the local rewrite on its copy.
>
> Suggested-by: Oleg Nesterov <oleg@redhat.com>

Ah, sorry... I only suggested to change the signature of send_signal_locked()
and thus has_si_pid_and_uid(). Perhaps a broader change makes sense too, but
this conflicts with another (under discussion) series:

	PATCH v2 3/3] signal: fix evasion of SA_IMMUTABLE signals
	https://lore.kernel.org/all/ajVD6ZmiSQLxjj57@redhat.com/

Now let me take another look at 1/2 ...

Oleg.

> Signed-off-by: Bradley Morgan <include@grrlz.net>
> ---
> Changes since v1:
> - New patch from Oleg's suggestion.
> - Link to Oleg's suggestion:
>   https://lore.kernel.org/all/0873AC4A-3CB2-4F7B-BFE6-75D855AD22DC@grrlz.net/T/#m5f8a2d54928efff41de539969b68149e1ec5fca4
> 
>  include/linux/signal.h        |  2 +-
>  include/trace/events/signal.h |  4 ++--
>  kernel/signal.c               | 20 +++++++++++---------
>  3 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/signal.h b/include/linux/signal.h
> index f19816832f05..a1ba8c5973c6 100644
> --- a/include/linux/signal.h
> +++ b/include/linux/signal.h
> @@ -283,7 +283,7 @@ extern int do_send_sig_info(int sig, struct kernel_siginfo *info,
>  				struct task_struct *p, enum pid_type type);
>  extern int group_send_sig_info(int sig, struct kernel_siginfo *info,
>  			       struct task_struct *p, enum pid_type type);
> -extern int send_signal_locked(int sig, struct kernel_siginfo *info,
> +extern int send_signal_locked(int sig, const struct kernel_siginfo *info,
>  			      struct task_struct *p, enum pid_type type);
>  extern int sigprocmask(int, sigset_t *, sigset_t *);
>  extern void set_current_blocked(sigset_t *);
> diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
> index 1db7e4b07c01..05a46135ee34 100644
> --- a/include/trace/events/signal.h
> +++ b/include/trace/events/signal.h
> @@ -49,8 +49,8 @@ enum {
>   */
>  TRACE_EVENT(signal_generate,
>  
> -	TP_PROTO(int sig, struct kernel_siginfo *info, struct task_struct *task,
> -			int group, int result),
> +	TP_PROTO(int sig, const struct kernel_siginfo *info,
> +		 struct task_struct *task, int group, int result),
>  
>  	TP_ARGS(sig, info, task, group, result),
>  
> diff --git a/kernel/signal.c b/kernel/signal.c
> index d72d9be3a992..26e8b8e1d03c 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1037,7 +1037,7 @@ static inline bool legacy_queue(struct sigpending *signals, int sig)
>  	return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
>  }
>  
> -static int __send_signal_locked(int sig, struct kernel_siginfo *info,
> +static int __send_signal_locked(int sig, const struct kernel_siginfo *info,
>  				struct task_struct *t, enum pid_type type, bool force)
>  {
>  	struct sigpending *pending;
> @@ -1154,7 +1154,7 @@ static int __send_signal_locked(int sig, struct kernel_siginfo *info,
>  	return ret;
>  }
>  
> -static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
> +static inline bool has_si_pid_and_uid(const struct kernel_siginfo *info)
>  {
>  	bool ret = false;
>  	switch (siginfo_layout(info->si_signo, info->si_code)) {
> @@ -1178,10 +1178,11 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
>  	return ret;
>  }
>  
> -int send_signal_locked(int sig, struct kernel_siginfo *info,
> +int send_signal_locked(int sig, const struct kernel_siginfo *info,
>  		       struct task_struct *t, enum pid_type type)
>  {
>  	struct kernel_siginfo rewritten;
> +	const struct kernel_siginfo *send_info = info;
>  	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
>  	bool force = false;
>  
> @@ -1196,26 +1197,27 @@ int send_signal_locked(int sig, struct kernel_siginfo *info,
>  		struct user_namespace *t_user_ns;
>  
>  		rewritten = *info;
> -		info = &rewritten;
> +		send_info = &rewritten;
>  
>  		rcu_read_lock();
>  		t_user_ns = task_cred_xxx(t, user_ns);
>  		if (current_user_ns() != t_user_ns) {
> -			kuid_t uid = make_kuid(current_user_ns(), info->si_uid);
> -			info->si_uid = from_kuid_munged(t_user_ns, uid);
> +			kuid_t uid = make_kuid(current_user_ns(), rewritten.si_uid);
> +
> +			rewritten.si_uid = from_kuid_munged(t_user_ns, uid);
>  		}
>  		rcu_read_unlock();
>  
>  		/* A kernel generated signal? */
> -		force = (info->si_code == SI_KERNEL);
> +		force = (rewritten.si_code == SI_KERNEL);
>  
>  		/* From an ancestor pid namespace? */
>  		if (!task_pid_nr_ns(current, task_active_pid_ns(t))) {
> -			info->si_pid = 0;
> +			rewritten.si_pid = 0;
>  			force = true;
>  		}
>  	}
> -	return __send_signal_locked(sig, info, t, type, force);
> +	return __send_signal_locked(sig, send_info, t, type, force);
>  }
>  
>  static void print_fatal_signal(int signr)
> -- 
> 2.53.0
> 

Re: [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites

[Oleg Nesterov]

Add Eric.

OK, I agree, it seems we need a simple fix.

Acked-by: Oleg Nesterov <oleg@redhat.com>

-------------------------------------------------------------------------
But let me add some "offtopic" notes... Why do we actually need this fix?

kill_something_info(). But at first glance sys_kill/kill_something_info
can simply use SEND_SIG_NOINFO? If yes, this makes sense anyway, I will
re-check...

do_pidfd_send_signal(PIDFD_SIGNAL_PROCESS_GROUP) allows to call
kill_pgrp_info() if si_code < 0... Not that I think this would be better,
but we could move this "rewrite" logic into __kill_pgrp_info()...

Anything else needs this change? Most probably yes, but after the quick
grep I don't see other group senders with !is_si_special(info).

Eric, what do you think?

Oleg.

On 06/22, Bradley Morgan wrote:
>
> send_signal_locked() rewrites sender ids for the target namespace.
> Group sends reuse the same siginfo, so one recipient can affect the
> next.
>
> Copy the siginfo before changing it.
>
> Fixes: 7a0cf094944e ("signal: Correct namespace fixups of si_pid and si_uid")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bradley Morgan <include@grrlz.net>
> ---
> Changes since v1:
> - No code changes in this patch.
> - Add patch 2 for Oleg's const suggestion.
> - Link to v1:
>   https://lore.kernel.org/all/0873AC4A-3CB2-4F7B-BFE6-75D855AD22DC@grrlz.net/T/#m89955d13f10807c316d34cc76680d690a2d95b31
>
>  kernel/signal.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index b9fc7be1a169..d72d9be3a992 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1181,6 +1181,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
>  int send_signal_locked(int sig, struct kernel_siginfo *info,
>  		       struct task_struct *t, enum pid_type type)
>  {
> +	struct kernel_siginfo rewritten;
>  	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
>  	bool force = false;
>
> @@ -1194,6 +1195,9 @@ int send_signal_locked(int sig, struct kernel_siginfo *info,
>  		/* SIGKILL and SIGSTOP is special or has ids */
>  		struct user_namespace *t_user_ns;
>
> +		rewritten = *info;
> +		info = &rewritten;
> +
>  		rcu_read_lock();
>  		t_user_ns = task_cred_xxx(t, user_ns);
>  		if (current_user_ns() != t_user_ns) {
> --
> 2.53.0
>

Re: [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo

[Bradley Morgan]

On June 23, 2026 11:39:05 AM GMT+01:00, Oleg Nesterov <oleg@redhat.com>
wrote:
>On 06/22, Bradley Morgan wrote:
>>
>> send_signal_locked() should not change the caller's siginfo. Make that
>> part of the type and keep the local rewrite on its copy.
>>
>> Suggested-by: Oleg Nesterov <oleg@redhat.com>
>
>Ah, sorry... I only suggested to change the signature of
>send_signal_locked()
>and thus has_si_pid_and_uid(). Perhaps a broader change makes sense too,
>but
>this conflicts with another (under discussion) series:
>
>	PATCH v2 3/3] signal: fix evasion of SA_IMMUTABLE signals
>	https://lore.kernel.org/all/ajVD6ZmiSQLxjj57@redhat.com/
>
>Now let me take another look at 1/2 ...
>
>Oleg.

Aww. My bad.

You may keep this shelved in case :)


>> Signed-off-by: Bradley Morgan <include@grrlz.net>
>> ---
>> Changes since v1:
>> - New patch from Oleg's suggestion.
>> - Link to Oleg's suggestion:
>>  
>https://lore.kernel.org/all/0873AC4A-3CB2-4F7B-BFE6-75D855AD22DC@grrlz.net/T/#m5f8a2d54928efff41de539969b68149e1ec5fca4
>> 
>>  include/linux/signal.h        |  2 +-
>>  include/trace/events/signal.h |  4 ++--
>>  kernel/signal.c               | 20 +++++++++++---------
>>  3 files changed, 14 insertions(+), 12 deletions(-)
>> 
>> diff --git a/include/linux/signal.h b/include/linux/signal.h
>> index f19816832f05..a1ba8c5973c6 100644
>> --- a/include/linux/signal.h
>> +++ b/include/linux/signal.h
>> @@ -283,7 +283,7 @@ extern int do_send_sig_info(int sig, struct
>kernel_siginfo *info,
>>  				struct task_struct *p, enum pid_type type);
>>  extern int group_send_sig_info(int sig, struct kernel_siginfo *info,
>>  			       struct task_struct *p, enum pid_type type);
>> -extern int send_signal_locked(int sig, struct kernel_siginfo *info,
>> +extern int send_signal_locked(int sig, const struct kernel_siginfo
>*info,
>>  			      struct task_struct *p, enum pid_type type);
>>  extern int sigprocmask(int, sigset_t *, sigset_t *);
>>  extern void set_current_blocked(sigset_t *);
>> diff --git a/include/trace/events/signal.h
>b/include/trace/events/signal.h
>> index 1db7e4b07c01..05a46135ee34 100644
>> --- a/include/trace/events/signal.h
>> +++ b/include/trace/events/signal.h
>> @@ -49,8 +49,8 @@ enum {
>>   */
>>  TRACE_EVENT(signal_generate,
>>  
>> -	TP_PROTO(int sig, struct kernel_siginfo *info, struct task_struct *task,
>> -			int group, int result),
>> +	TP_PROTO(int sig, const struct kernel_siginfo *info,
>> +		 struct task_struct *task, int group, int result),
>>  
>>  	TP_ARGS(sig, info, task, group, result),
>>  
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index d72d9be3a992..26e8b8e1d03c 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -1037,7 +1037,7 @@ static inline bool legacy_queue(struct sigpending
>*signals, int sig)
>>  	return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
>>  }
>>  
>> -static int __send_signal_locked(int sig, struct kernel_siginfo *info,
>> +static int __send_signal_locked(int sig, const struct kernel_siginfo
>*info,
>>  				struct task_struct *t, enum pid_type type, bool force)
>>  {
>>  	struct sigpending *pending;
>> @@ -1154,7 +1154,7 @@ static int __send_signal_locked(int sig, struct
>kernel_siginfo *info,
>>  	return ret;
>>  }
>>  
>> -static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
>> +static inline bool has_si_pid_and_uid(const struct kernel_siginfo
>*info)
>>  {
>>  	bool ret = false;
>>  	switch (siginfo_layout(info->si_signo, info->si_code)) {
>> @@ -1178,10 +1178,11 @@ static inline bool has_si_pid_and_uid(struct
>kernel_siginfo *info)
>>  	return ret;
>>  }
>>  
>> -int send_signal_locked(int sig, struct kernel_siginfo *info,
>> +int send_signal_locked(int sig, const struct kernel_siginfo *info,
>>  		       struct task_struct *t, enum pid_type type)
>>  {
>>  	struct kernel_siginfo rewritten;
>> +	const struct kernel_siginfo *send_info = info;
>>  	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
>>  	bool force = false;
>>  
>> @@ -1196,26 +1197,27 @@ int send_signal_locked(int sig, struct
>kernel_siginfo *info,
>>  		struct user_namespace *t_user_ns;
>>  
>>  		rewritten = *info;
>> -		info = &rewritten;
>> +		send_info = &rewritten;
>>  
>>  		rcu_read_lock();
>>  		t_user_ns = task_cred_xxx(t, user_ns);
>>  		if (current_user_ns() != t_user_ns) {
>> -			kuid_t uid = make_kuid(current_user_ns(), info->si_uid);
>> -			info->si_uid = from_kuid_munged(t_user_ns, uid);
>> +			kuid_t uid = make_kuid(current_user_ns(), rewritten.si_uid);
>> +
>> +			rewritten.si_uid = from_kuid_munged(t_user_ns, uid);
>>  		}
>>  		rcu_read_unlock();
>>  
>>  		/* A kernel generated signal? */
>> -		force = (info->si_code == SI_KERNEL);
>> +		force = (rewritten.si_code == SI_KERNEL);
>>  
>>  		/* From an ancestor pid namespace? */
>>  		if (!task_pid_nr_ns(current, task_active_pid_ns(t))) {
>> -			info->si_pid = 0;
>> +			rewritten.si_pid = 0;
>>  			force = true;
>>  		}
>>  	}
>> -	return __send_signal_locked(sig, info, t, type, force);
>> +	return __send_signal_locked(sig, send_info, t, type, force);
>>  }
>>  
>>  static void print_fatal_signal(int signr)
>> -- 
>> 2.53.0
>> 
>
>

Thanks!

Re: [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites

[Eric W. Biederman]

Oleg Nesterov <oleg@redhat.com> writes:

> Add Eric.
>
> OK, I agree, it seems we need a simple fix.
>
> Acked-by: Oleg Nesterov <oleg@redhat.com>
>
> -------------------------------------------------------------------------
> But let me add some "offtopic" notes... Why do we actually need this fix?
>
> kill_something_info(). But at first glance sys_kill/kill_something_info
> can simply use SEND_SIG_NOINFO? If yes, this makes sense anyway, I will
> re-check...
>
> do_pidfd_send_signal(PIDFD_SIGNAL_PROCESS_GROUP) allows to call
> kill_pgrp_info() if si_code < 0... Not that I think this would be better,
> but we could move this "rewrite" logic into __kill_pgrp_info()...
>
> Anything else needs this change? Most probably yes, but after the quick
> grep I don't see other group senders with !is_si_special(info).
>
> Eric, what do you think?

So I think tracing the basic kill syscall is interesting.

It uses an explicit siginfo.  It does that so it can choose
between setting si_code to SI_TKILL and SI_USER.

If the signal number is -1 it sends to every process in the
system (or at least the pid namespace).

That will require translation.

So either we need to add another special siginfo value to handle
SI_TKILL, or we need to fix this the way that was suggested.

I suspect just fixing send_signal_locked looks the easiest,
especially if you make the siginfo parameter const.

It would likely help to have a self test that detects the problem before
this is fixed and passes afterwards so we have some chance of detecting
if someone makes a similar mistake in the future.

Eric

Re: [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites

[Oleg Nesterov]

On 06/24, Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@redhat.com> writes:
>
> > Add Eric.
> >
> > OK, I agree, it seems we need a simple fix.
> >
> > Acked-by: Oleg Nesterov <oleg@redhat.com>
> >
> > -------------------------------------------------------------------------
> > But let me add some "offtopic" notes... Why do we actually need this fix?
> >
> > kill_something_info(). But at first glance sys_kill/kill_something_info
> > can simply use SEND_SIG_NOINFO? If yes, this makes sense anyway, I will
> > re-check...

....

> So I think tracing the basic kill syscall is interesting.
>
> It uses an explicit siginfo.  It does that so it can choose
> between setting si_code to SI_TKILL and SI_USER.
>
> If the signal number is -1 it sends to every process in the
> system (or at least the pid namespace).
>
> That will require translation.

Most probably I was wrong, I didn't try to re-check yet.

But at first glance kill_something_info() never use SI_TKILL, and
__send_signal_locked(SEND_SIG_NOINFO) will do the necessary translation,
in this case si_pid/si_uid are the current task's pid/uid.

But again, I am not sure. Didn't have to to actually look at this code.

> I suspect just fixing send_signal_locked looks the easiest,
> especially if you make the siginfo parameter const.

Yes, agreed, and I have already acked this patch.

I think we can improve this unconditional rewrite later, on top of this fix.

Oleg.

Re: [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites

[Bradley Morgan]

On June 24, 2026 4:52:09 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com>
wrote:
>On 06/24, Eric W. Biederman wrote:
>>
>> Oleg Nesterov <oleg@redhat.com> writes:
>>
>> > Add Eric.
>> >
>> > OK, I agree, it seems we need a simple fix.
>> >
>> > Acked-by: Oleg Nesterov <oleg@redhat.com>
>> >
>> >
>-------------------------------------------------------------------------
>> > But let me add some "offtopic" notes... Why do we actually need this
>fix?
>> >
>> > kill_something_info(). But at first glance
>sys_kill/kill_something_info
>> > can simply use SEND_SIG_NOINFO? If yes, this makes sense anyway, I
>will
>> > re-check...
>
>....
>
>> So I think tracing the basic kill syscall is interesting.
>>
>> It uses an explicit siginfo.  It does that so it can choose
>> between setting si_code to SI_TKILL and SI_USER.
>>
>> If the signal number is -1 it sends to every process in the
>> system (or at least the pid namespace).
>>
>> That will require translation.
>
>Most probably I was wrong, I didn't try to re-check yet.
>
>But at first glance kill_something_info() never use SI_TKILL, and
>__send_signal_locked(SEND_SIG_NOINFO) will do the necessary translation,
>in this case si_pid/si_uid are the current task's pid/uid.
>
>But again, I am not sure. Didn't have to to actually look at this code.
>
>> I suspect just fixing send_signal_locked looks the easiest,
>> especially if you make the siginfo parameter const.
>
>Yes, agreed, and I have already acked this patch.
>
>I think we can improve this unconditional rewrite later, on top of this
>fix.
>
>Oleg.
>
>

Hey you two, sorry to impede in your conversation, but could we write
your "conflicting" patch over my Patch 2?

It's fine if you don't want to, it kind of kills two birds with one stone.

Thanks!

Re: [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites

[Oleg Nesterov]

On 06/24, Bradley Morgan wrote:
>
> Hey you two, sorry to impede in your conversation, but could we write
> your "conflicting" patch over my Patch 2?
>
> It's fine if you don't want to, it kind of kills two birds with one stone.

No, sorry, I don't ;) at least right now. Because I don't really like the
changes it adds into send_signal_locked(). But perhaps I didn't read it
carefully.

Can we return to it later? There is another reason... Currently I am very
busy but I am thinking about another change on top of your 1/2. Something
like below. Not sure it makes a lot of sense though.

Eric, do you think this optimization on top of 1/2 makes sense?

Oleg.

int send_signal_locked(int sig, struct kernel_siginfo *info,
		       struct task_struct *t, enum pid_type type)
{
	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
	struct kernel_siginfo __info;
	bool force = false;

	if (info == SEND_SIG_NOINFO) {
		/* Force if sent from an ancestor pid namespace */
		force = !task_pid_nr_ns(current, task_active_pid_ns(t));
	} else if (info == SEND_SIG_PRIV) {
		/* Don't ignore kernel generated signals */
		force = true;
	} else if (has_si_pid_and_uid(info)) {
		/* SIGKILL and SIGSTOP is special or has ids */
		struct user_namespace *t_user_ns;

#ifdef CONFIG_USER_NS
		rcu_read_lock();
		t_user_ns = task_cred_xxx(t, user_ns);
		if (current_user_ns() != t_user_ns) {
			__info = *info;
			info = &__info;
			kuid_t uid = make_kuid(current_user_ns(), info->si_uid);
			info->si_uid = from_kuid_munged(t_user_ns, uid);
		}
		rcu_read_unlock();
#endif
		/* A kernel generated signal? */
		force = (info->si_code == SI_KERNEL);

#ifdef CONFIG_PID_NS
		/* From an ancestor pid namespace? */
		if (!task_pid_nr_ns(current, task_active_pid_ns(t))) {
			if (info != &__info) {
				__info = *info;
				info = &__info;
			}
			info->si_pid = 0;
			force = true;
		}
#endif
	}
	return __send_signal_locked(sig, info, t, type, force);
}