Re: [PATCH -mm 1/1] signal: avoid unconditional siginfo copy in send_signal_locked()

[Bradley Morgan]

On July 4, 2026 3:34:42 PM GMT+01:00, Oleg Nesterov <oleg@xxxxxxxxxx>
wrote:
>send_signal_locked() unconditionally copies siginfo before the namespace
>translation to avoid corrupting a shared siginfo.
>
>Not that I think this can actually hurt performance-wise, just it doesn't
>look clean to me; the copy is only needed in the unlikely case when the
>translation will actually change something.
>
>Defer it to the two cases where si_pid/si_uid are rewritten, and while at
>it add #ifdef's just for completeness.
>
>Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
>---
> kernel/signal.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
>diff --git a/kernel/signal.c b/kernel/signal.c
>index 041498ff835e..0f509ee4f42d 100644
>--- a/kernel/signal.c
>+++ b/kernel/signal.c
>@@ -1181,7 +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;
>+	struct kernel_siginfo __maybe_unused rewritten;
> 	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
> 	bool force = false;
> 
>@@ -1193,27 +1193,34 @@ int send_signal_locked(int sig, struct kernel_siginfo *info,
> 		force = true;
> 	} else if (has_si_pid_and_uid(info)) {
> 		/* SIGKILL and SIGSTOP is special or has ids */
>+#ifdef CONFIG_USER_NS
> 		struct user_namespace *t_user_ns;
>-
>-		rewritten = *info;
>-		info = &rewritten;
>+		kuid_t uid;
> 
> 		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);
>+			rewritten = *info;
>+			info = &rewritten;
>+			uid = make_kuid(current_user_ns(), info->si_uid);
>+			rewritten.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))) {
>-			info->si_pid = 0;
>+			if (info != &rewritten) {
>+				rewritten = *info;
>+				info = &rewritten;
>+			}
>+			rewritten.si_pid = 0;
> 			force = true;
> 		}
>+#endif
> 	}
> 	return __send_signal_locked(sig, info, t, type, force);
> }
>
hi oleg, 

thanks for the patch!

Reviewed-by: Bradley Morgan <include@xxxxxxxxx>
Thanks!