* [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites [not found] <20260622164029.11474-1-include@grrlz.net> @ 2026-06-22 20:25 ` Bradley Morgan 2026-06-23 11:37 ` Oleg Nesterov 2026-06-22 20:25 ` [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo Bradley Morgan 1 sibling, 1 reply; 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
* Re: [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites 2026-06-22 20:25 ` [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites Bradley Morgan @ 2026-06-23 11:37 ` Oleg Nesterov 0 siblings, 0 replies; 5+ messages in thread From: Oleg Nesterov @ 2026-06-23 11:37 UTC (permalink / raw) To: Bradley Morgan, Eric W. Biederman Cc: Christian Brauner, 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, stable 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo [not found] <20260622164029.11474-1-include@grrlz.net> 2026-06-22 20:25 ` [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites Bradley Morgan @ 2026-06-22 20:25 ` Bradley Morgan 2026-06-23 10:39 ` Oleg Nesterov 1 sibling, 1 reply; 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
* Re: [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo 2026-06-22 20:25 ` [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo Bradley Morgan @ 2026-06-23 10:39 ` Oleg Nesterov 2026-06-23 14:49 ` Bradley Morgan 0 siblings, 1 reply; 5+ messages in thread From: Oleg Nesterov @ 2026-06-23 10:39 UTC (permalink / raw) To: Bradley Morgan Cc: Christian Brauner, 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 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo 2026-06-23 10:39 ` Oleg Nesterov @ 2026-06-23 14:49 ` Bradley Morgan 0 siblings, 0 replies; 5+ messages in thread From: Bradley Morgan @ 2026-06-23 14:49 UTC (permalink / raw) To: Oleg Nesterov Cc: Christian Brauner, 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 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! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-23 14:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260622164029.11474-1-include@grrlz.net>
2026-06-22 20:25 ` [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites Bradley Morgan
2026-06-23 11:37 ` Oleg Nesterov
2026-06-22 20:25 ` [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo Bradley Morgan
2026-06-23 10:39 ` Oleg Nesterov
2026-06-23 14:49 ` Bradley Morgan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox