Re: [PATCH v2] sysctl: move the "cad_pid" entry from pid_table[] to kern_reboot_table[]

[Bradley Morgan]

On July 20, 2026 12:13:43 PM GMT+01:00, Oleg Nesterov <oleg@xxxxxxxxxx>
wrote:
>cad_pid is global, and kill_cad_pid() is only used in the root namespace.
>
>However, due to pid_table_root_permissions(), a non-root user can unshare
>pid/user namespaces and modify it from the child namespace. This makes no
>sense and is simply wrong.
>
>Move it to kern_reboot_table[] where it logically belongs; this ensures
>that only GLOBAL_ROOT_UID can read/modify this sysctl.
>
>Note that this patch doesn't preserve "#ifdef CONFIG_PROC_SYSCTL" around
>the "cad_pid"; CONFIG_PROC_SYSCTL selects CONFIG_SYSCTL, so it is always
>set when kern_reboot_table[] is compiled.

Worlds smallest nit: Could you add the patch this was inspired by in the
description?

E.g:
<proposal>
This patch was inspired from Cens patch fixing a UAF in cid_pid [1]
 
[1] 
https://lore.kernel.org/all/20260719155842.7069-2-blbllhy@xxxxxxxxx/
<Proposal-end>


sadly no cover letter.

(Note: Andrew could add this to the description, patch is still good, tag
is still valid)


>Cc: stable@xxxxxxxxxxxxxxx
>Fixes: e054bcbe7e7a ("sysctl: move cad_pid into kernel/pid.c")
>Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
>Acked-by: Alexey Gladkov <legion@xxxxxxxxxx>
>Reviewed-by: Bradley Morgan <include@xxxxxxxxx>
>Reviewed-by: Pavel Tikhomirov <ptikhomirov@xxxxxxxxxxxxx>
>---
> kernel/pid.c    | 31 -------------------------------
> kernel/reboot.c | 29 +++++++++++++++++++++++++++++
> 2 files changed, 29 insertions(+), 31 deletions(-)
>
>diff --git a/kernel/pid.c b/kernel/pid.c
>index f55189a3d07d..1c27e63fa0ad 100644
>--- a/kernel/pid.c
>+++ b/kernel/pid.c
>@@ -764,29 +764,6 @@ static struct ctl_table_root pid_table_root = {
> 	.set_ownership	= pid_table_root_set_ownership,
> };
> 
>-static int proc_do_cad_pid(const struct ctl_table *table, int write, void *buffer,
>-		size_t *lenp, loff_t *ppos)
>-{
>-	struct pid *new_pid;
>-	pid_t tmp_pid;
>-	int r;
>-	struct ctl_table tmp_table = *table;
>-
>-	tmp_pid = pid_vnr(cad_pid);
>-	tmp_table.data = &tmp_pid;
>-
>-	r = proc_dointvec(&tmp_table, write, buffer, lenp, ppos);
>-	if (r || !write)
>-		return r;
>-
>-	new_pid = find_get_pid(tmp_pid);
>-	if (!new_pid)
>-		return -ESRCH;
>-
>-	put_pid(xchg(&cad_pid, new_pid));
>-	return 0;
>-}
>-
> static const struct ctl_table pid_table[] = {
> 	{
> 		.procname	= "pid_max",
>@@ -797,14 +774,6 @@ static const struct ctl_table pid_table[] = {
> 		.extra1		= &pid_max_min,
> 		.extra2		= &pid_max_max,
> 	},
>-#ifdef CONFIG_PROC_SYSCTL
>-	{
>-		.procname	= "cad_pid",
>-		.maxlen		= sizeof(int),
>-		.mode		= 0600,
>-		.proc_handler	= proc_do_cad_pid,
>-	},
>-#endif
> };
> #endif
> 
>diff --git a/kernel/reboot.c b/kernel/reboot.c
>index 695c33e75efd..f070c5c1103a 100644
>--- a/kernel/reboot.c
>+++ b/kernel/reboot.c
>@@ -1366,6 +1366,29 @@ static struct attribute *reboot_attrs[] = {
> };
> 
> #ifdef CONFIG_SYSCTL
>+static int proc_do_cad_pid(const struct ctl_table *table, int write, void *buffer,
>+			   size_t *lenp, loff_t *ppos)
>+{
>+	struct ctl_table tmp_table = *table;
>+	struct pid *new_pid;
>+	pid_t tmp_pid;
>+	int r;
>+
>+	tmp_pid = pid_vnr(cad_pid);
>+	tmp_table.data = &tmp_pid;
>+
>+	r = proc_dointvec(&tmp_table, write, buffer, lenp, ppos);
>+	if (r || !write)
>+		return r;
>+
>+	new_pid = find_get_pid(tmp_pid);
>+	if (!new_pid)
>+		return -ESRCH;
>+
>+	put_pid(xchg(&cad_pid, new_pid));
>+	return 0;
>+}
>+
> static const struct ctl_table kern_reboot_table[] = {
> 	{
> 		.procname       = "poweroff_cmd",
>@@ -1381,6 +1404,12 @@ static const struct ctl_table kern_reboot_table[] = {
> 		.mode           = 0644,
> 		.proc_handler   = proc_dointvec,
> 	},
>+	{
>+		.procname	= "cad_pid",
>+		.maxlen		= sizeof(int),
>+		.mode		= 0600,
>+		.proc_handler	= proc_do_cad_pid,
>+	},
> };
> 
> static void __init kernel_reboot_sysctls_init(void)
>

Thanks!