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

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

[Oleg Nesterov]

On top of

	signal-avoid-shared-siginfo-namespace-rewrites.patch

in -mm tree.

To clarify, this (simple) change looks like a cleanup to me even if it slightly
complicates send_signal_locked(). But cleanups are always subjective, so I won't
argue if anyone thinks otherwise.

To remind, I think we should shift this namespace translation into
__send_signal_locked() -> copy_siginfo() path, but this needs something like
[PATCH v2 2/3] signal: turn the "bool force" arg of __send_signal_locked() into "int flags"
https://lore.kernel.org/all/ajVDzniRXROuyRgD@redhat.com/

Oleg.

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

[Oleg Nesterov]

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@redhat.com>
---
 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);
 }
-- 
2.52.0

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@redhat.com>
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@redhat.com>
>---
> 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@grrlz.net>
Thanks!

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

[Christian Brauner]

On Sat, Jul 04, 2026 at 04:34:42PM +0200, Oleg Nesterov 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@redhat.com>
> ---

Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>