[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