On June 23, 2026 11:39:05 AM GMT+01:00, Oleg Nesterov <[email protected]> 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 <[email protected]> > >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/[email protected]/ > >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 <[email protected]> >> --- >> Changes since v1: >> - New patch from Oleg's suggestion. >> - Link to Oleg's suggestion: >> >https://lore.kernel.org/all/[email protected]/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!
