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

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

[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: stable@vger.kernel.org
Fixes: e054bcbe7e7a ("sysctl: move cad_pid into kernel/pid.c")
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Alexey Gladkov <legion@kernel.org>
Reviewed-by: Bradley Morgan <include@grrlz.net>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 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)
-- 
2.52.0

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

[Oleg Nesterov]

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.

On 07/20, Oleg Nesterov 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.
>
> Cc: stable@vger.kernel.org
> Fixes: e054bcbe7e7a ("sysctl: move cad_pid into kernel/pid.c")
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Alexey Gladkov <legion@kernel.org>
> Reviewed-by: Bradley Morgan <include@grrlz.net>
> Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>  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)
> --
> 2.52.0
>

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@redhat.com>
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@gmail.com/
<Proposal-end>


sadly no cover letter.

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


>Cc: stable@vger.kernel.org
>Fixes: e054bcbe7e7a ("sysctl: move cad_pid into kernel/pid.c")
>Signed-off-by: Oleg Nesterov <oleg@redhat.com>
>Acked-by: Alexey Gladkov <legion@kernel.org>
>Reviewed-by: Bradley Morgan <include@grrlz.net>
>Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>---
> 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!

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@grrlz.net>
wrote:
>On July 20, 2026 12:13:43 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com>
>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@gmail.com/
><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@vger.kernel.org
>>Fixes: e054bcbe7e7a ("sysctl: move cad_pid into kernel/pid.c")
>>Signed-off-by: Oleg Nesterov <oleg@redhat.com>
>>Acked-by: Alexey Gladkov <legion@kernel.org>
>>Reviewed-by: Bradley Morgan <include@grrlz.net>
>>Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>>---
>> 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!

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

[Oleg Nesterov]

On 07/20, Bradley Morgan wrote:
>
> On July 20, 2026 12:13:43 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com>
> 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]

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.