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

PATCH Applied Oleg Nesterov 2 participants 5 messages Jul 20

lore

Share & tools

lore

Oleg Nesterov

author

2026-07-20 11:13 UTC #0 lore raw

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

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.

Cc [email protected]

Fixes e054bcbe7e7a ("sysctl: move cad_pid into kernel/pid.c")

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Alexey Gladkov <[email protected]>

Reviewed-by Bradley Morgan <[email protected]>

Reviewed-by Pavel Tikhomirov <[email protected]>

---
 kernel/pid.c    | 31 -------------------------------
 kernel/reboot.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 31 deletions(-)

kernel/pid.c +0 ~~-31~~

index f55189a3d07d..1c27e63fa0ad 100644
@@ -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

kernel/reboot.c +29 -0

index 695c33e75efd..f070c5c1103a 100644
@@ -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)

Signature · 1 line

2.52.0

Oleg Nesterov

author

2026-07-20 11:35 UTC #1 lore raw

OK, sashiko correctly warns about pre-existing problem

	https://sashiko.dev/#/patchset/al4C572uhLdBvyzH%40redhat.com

but this is exactly what the patches from Cen should fix.

Oleg.

Quoted 124 lines · Oleg Nesterov

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.

Cc [email protected]

Fixes e054bcbe7e7a ("sysctl: move cad_pid into kernel/pid.c")

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Alexey Gladkov <[email protected]>

Reviewed-by Bradley Morgan <[email protected]>

Reviewed-by Pavel Tikhomirov <[email protected]>

---
 kernel/pid.c    | 31 -------------------------------
 kernel/reboot.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 31 deletions(-)

kernel/pid.c +0 ~~-31~~

index f55189a3d07d..1c27e63fa0ad 100644
@@ -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

kernel/reboot.c +29 -1

index 695c33e75efd..f070c5c1103a 100644
@@ -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)
--

2.52.0

Bradley Morgan

2026-07-20 11:46 UTC #2 lore raw

In reply to Oleg Nesterov

Quoted 6 lines · Oleg Nesterov

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.

Quoted 6 lines

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/[email protected]/
<Proposal-end>

sadly no cover letter.

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

Quoted 98 lines

Cc [email protected]

Fixes e054bcbe7e7a ("sysctl: move cad_pid into kernel/pid.c")

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Alexey Gladkov <[email protected]>

Reviewed-by Bradley Morgan <[email protected]>

Reviewed-by Pavel Tikhomirov <[email protected]>

---
kernel/pid.c | 31 -------------------------------
kernel/reboot.c | 29 +++++++++++++++++++++++++++++

2 files changed, 29 insertions(+), 31 deletions(-)

kernel/pid.c +0 ~~-31~~

index f55189a3d07d..1c27e63fa0ad 100644
@@ -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

kernel/reboot.c +23 -0

index 695c33e75efd..f070c5c1103a 100644
@@ -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,

kernel/reboot.c static const struct ctl_table kern_reboot_table[] = { · line 1415

Quoted 10 lines

kernel/reboot.c +6 -0

 	},
+	{
+		.procname	= "cad_pid",
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_do_cad_pid,
+	},
 };
 
 static void __init kernel_reboot_sysctls_init(void)

Thanks!

Bradley Morgan

2026-07-20 11:49 UTC #3 lore raw

Quoted 27 lines · Bradley Morgan

Quoted 6 lines · Oleg Nesterov
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.
Quoted 6 lines
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/[email protected]/
<Proposal-end>

Quoted 5 lines

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

Quoted 109 lines

Quoted 16 lines

Cc [email protected]
Fixes e054bcbe7e7a ("sysctl: move cad_pid into kernel/pid.c")
Signed-off-by Oleg Nesterov <[email protected]>
Acked-by Alexey Gladkov <[email protected]>
Reviewed-by Bradley Morgan <[email protected]>
Reviewed-by Pavel Tikhomirov <[email protected]>
---
kernel/pid.c | 31 -------------------------------
kernel/reboot.c | 29 +++++++++++++++++++++++++++++
2 files changed, 29 insertions(+), 31 deletions(-)
kernel/pid.c +0 -0
index f55189a3d07d..1c27e63fa0ad 100644

kernel/pid.c static struct ctl_table_root pid_table_root = { · line 767

Quoted 4 lines

kernel/pid.c +0 -1

@@ -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,

Quoted 45 lines

kernel/pid.c +0 ~~-30~~

-		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

kernel/reboot.c +0 -0

index 695c33e75efd..f070c5c1103a 100644

kernel/reboot.c static struct attribute *reboot_attrs[] = { · line 1369

Quoted 4 lines

kernel/reboot.c +1 -0

@@ -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,

kernel/reboot.c static const struct ctl_table kern_reboot_table[]

Quoted 26 lines

kernel/reboot.c +22 -0

+			   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[]

= {

Quoted 11 lines

kernel/reboot.c +6 -0

 		.mode           = 0644,
 		.proc_handler   = proc_dointvec,
 	},
+	{
+		.procname	= "cad_pid",
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_do_cad_pid,
+	},
 };

Quoted 4 lines

kernel/reboot.c static const struct ctl_table kern_reboot_table[] = { · line 1415
Quoted 2 lines
kernel/reboot.c +0 -0
 static void __init kernel_reboot_sysctls_init(void)
Thanks!

Thanks!

Oleg Nesterov

author

2026-07-20 11:57 UTC #4 lore raw

In reply to Bradley Morgan

Quoted 16 lines · Bradley Morgan

Quoted 6 lines · Oleg Nesterov
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.
Quoted 6 lines
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.

Quoted 6 lines

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]

This patch was inspired by sashiko.dev who found this problem. Not by the patches from Cen.

I'd be happy to add Reported-by: sashiko.dev but I don't think I can.

Oleg.