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

[Bradley Morgan]

On July 20, 2026 12:46:42 PM GMT+01:00, Bradley Morgan <include@xxxxxxxxx>
wrote:
>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)
>

Sorry, s/cid_pid/cad_pid

>>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!

Thanks!