* Re: [PATCH] signal: avoid shared siginfo namespace rewrites
2026-06-22 16:40 [PATCH] signal: avoid shared siginfo namespace rewrites Bradley Morgan
@ 2026-06-22 17:46 ` Oleg Nesterov
2026-06-22 20:05 ` Bradley Morgan
2026-06-22 20:25 ` [PATCH v2 1/2] " Bradley Morgan
2026-06-22 20:25 ` [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo Bradley Morgan
2 siblings, 1 reply; 5+ messages in thread
From: Oleg Nesterov @ 2026-06-22 17:46 UTC (permalink / raw)
To: Bradley Morgan
Cc: Christian Brauner, ebiederm, Andrew Morton, Peter Zijlstra,
Adrian Huang, Marco Elver, Kexin Sun, Thomas Gleixner,
linux-kernel, stable
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites
2026-06-22 16:40 [PATCH] signal: avoid shared siginfo namespace rewrites Bradley Morgan
2026-06-22 17:46 ` Oleg Nesterov
@ 2026-06-22 20:25 ` Bradley Morgan
2026-06-22 20:25 ` [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo Bradley Morgan
2 siblings, 0 replies; 5+ messages in thread
From: Bradley Morgan @ 2026-06-22 20:25 UTC (permalink / raw)
To: Oleg Nesterov, Christian Brauner
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Peter Zijlstra, Marco Elver, Aleksandr Nogikh,
Thomas Gleixner, Adrian Huang, Kexin Sun, linux-kernel,
linux-trace-kernel, Bradley Morgan, stable
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo
2026-06-22 16:40 [PATCH] signal: avoid shared siginfo namespace rewrites Bradley Morgan
2026-06-22 17:46 ` Oleg Nesterov
2026-06-22 20:25 ` [PATCH v2 1/2] " Bradley Morgan
@ 2026-06-22 20:25 ` Bradley Morgan
2 siblings, 0 replies; 5+ messages in thread
From: Bradley Morgan @ 2026-06-22 20:25 UTC (permalink / raw)
To: Oleg Nesterov, Christian Brauner
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Peter Zijlstra, Marco Elver, Aleksandr Nogikh,
Thomas Gleixner, Adrian Huang, Kexin Sun, linux-kernel,
linux-trace-kernel, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread