[PATCH] ucounts: constify usage of ctl_table_header::ctl_table_arg

Thomas Weißschuh posted 1 patch 1 year, 9 months ago
kernel/ucount.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] ucounts: constify usage of ctl_table_header::ctl_table_arg
Posted by Thomas Weißschuh 1 year, 9 months ago
The sysctl core is preparing to only expose instances of
struct ctl_table as "const".
This will also affect the member
ctl_table_header::ctl_table_arg.

Prepare for that change to "struct ctl_table_header",
and already constify the usage of ctl_table_arg.

No functional change.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Motivation
==========

Moving structures containing function pointers into unmodifiable .rodata
prevents attackers or bugs from corrupting and diverting those pointers.

Also the "struct ctl_table" exposed by the sysctl core were never meant
to be mutated by users.

Process
=======

* Drop ctl_table modifications from the sysctl core ([0], in -next)
* Constify arguments to ctl_table_root::{set_ownership,permissions}
  ([1], in -next)
* Migrate users of "ctl_table_header::ctl_table_arg" to "const".
  (in progress, this patch)
* Afterwards convert "ctl_table_header::ctl_table_arg" itself to const.
  (to be done)
* Prepare helpers used to implement proc_handlers throughout the tree to
  use "const struct ctl_table *". ([2], in progress)
* Afterwards switch over all proc_handlers callbacks to use
  "const struct ctl_table *" in one commit. ([2], in progress)
  Only custom handlers will be affected, the big commit avoids a
  disruptive and messy transition phase.
* Switch over the internals of the sysctl core to "const struct ctl_table *" (to be done)
* Switch include/linux/sysctl.h to "const struct ctl_table *" (to be done)
* Transition instances of "struct ctl_table" through the tree to const (to be done)

A work-in-progress view containging all the outlined changes can be found at
https://git.sr.ht/~t-8ch/linux sysctl-constfy

[0] https://lore.kernel.org/lkml/20240322-sysctl-empty-dir-v2-0-e559cf8ec7c0@weissschuh.net/
[1] https://lore.kernel.org/lkml/20240315-sysctl-const-ownership-v3-0-b86680eae02e@weissschuh.net/
[2] https://lore.kernel.org/lkml/20240423-sysctl-const-handler-v3-0-e0beccb836e2@weissschuh.net/

Cc: Joel Granados <j.granados@samsung.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
---
 kernel/ucount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/ucount.c b/kernel/ucount.c
index 4aa6166cb856..d9e283600f5c 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -119,7 +119,7 @@ bool setup_userns_sysctls(struct user_namespace *ns)
 void retire_userns_sysctls(struct user_namespace *ns)
 {
 #ifdef CONFIG_SYSCTL
-	struct ctl_table *tbl;
+	const struct ctl_table *tbl;
 
 	tbl = ns->sysctls->ctl_table_arg;
 	unregister_sysctl_table(ns->sysctls);

---
base-commit: cf87f46fd34d6c19283d9625a7822f20d90b64a4
change-id: 20240511-sysctl-const-table-arg-ucount-75027ec3fbf4

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

Re: [PATCH] ucounts: constify usage of ctl_table_header::ctl_table_arg
Posted by Thomas Weißschuh 1 year, 9 months ago
On 2024-05-11 21:28:10+0000, Thomas Weißschuh wrote:
> The sysctl core is preparing to only expose instances of
> struct ctl_table as "const".
> This will also affect the member
> ctl_table_header::ctl_table_arg.
> 
> Prepare for that change to "struct ctl_table_header",
> and already constify the usage of ctl_table_arg.

Please disregard this patch.
Apparently Dave already picked it up as part of [0] through the
net-next tree, which makes sense the ctl_table_arg changes are in net/
for the largest part.

See commit bfa858f220ab ("sysctl: treewide: constify ctl_table_header::ctl_table_arg").

[0] https://lore.kernel.org/lkml/20240418-sysctl-const-table-arg-v2-1-4012abc31311@weissschuh.net/
> 
> No functional change.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Motivation
> ==========
> 
> Moving structures containing function pointers into unmodifiable .rodata
> prevents attackers or bugs from corrupting and diverting those pointers.
> 
> Also the "struct ctl_table" exposed by the sysctl core were never meant
> to be mutated by users.
> 
> Process
> =======
> 
> * Drop ctl_table modifications from the sysctl core ([0], in -next)
> * Constify arguments to ctl_table_root::{set_ownership,permissions}
>   ([1], in -next)
> * Migrate users of "ctl_table_header::ctl_table_arg" to "const".
>   (in progress, this patch)
> * Afterwards convert "ctl_table_header::ctl_table_arg" itself to const.
>   (to be done)
> * Prepare helpers used to implement proc_handlers throughout the tree to
>   use "const struct ctl_table *". ([2], in progress)
> * Afterwards switch over all proc_handlers callbacks to use
>   "const struct ctl_table *" in one commit. ([2], in progress)
>   Only custom handlers will be affected, the big commit avoids a
>   disruptive and messy transition phase.
> * Switch over the internals of the sysctl core to "const struct ctl_table *" (to be done)
> * Switch include/linux/sysctl.h to "const struct ctl_table *" (to be done)
> * Transition instances of "struct ctl_table" through the tree to const (to be done)
> 
> A work-in-progress view containging all the outlined changes can be found at
> https://git.sr.ht/~t-8ch/linux sysctl-constfy
> 
> [0] https://lore.kernel.org/lkml/20240322-sysctl-empty-dir-v2-0-e559cf8ec7c0@weissschuh.net/
> [1] https://lore.kernel.org/lkml/20240315-sysctl-const-ownership-v3-0-b86680eae02e@weissschuh.net/
> [2] https://lore.kernel.org/lkml/20240423-sysctl-const-handler-v3-0-e0beccb836e2@weissschuh.net/
> 
> Cc: Joel Granados <j.granados@samsung.com>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  kernel/ucount.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/ucount.c b/kernel/ucount.c
> index 4aa6166cb856..d9e283600f5c 100644
> --- a/kernel/ucount.c
> +++ b/kernel/ucount.c
> @@ -119,7 +119,7 @@ bool setup_userns_sysctls(struct user_namespace *ns)
>  void retire_userns_sysctls(struct user_namespace *ns)
>  {
>  #ifdef CONFIG_SYSCTL
> -	struct ctl_table *tbl;
> +	const struct ctl_table *tbl;
>  
>  	tbl = ns->sysctls->ctl_table_arg;
>  	unregister_sysctl_table(ns->sysctls);
> 
> ---
> base-commit: cf87f46fd34d6c19283d9625a7822f20d90b64a4
> change-id: 20240511-sysctl-const-table-arg-ucount-75027ec3fbf4
> 
> Best regards,
> -- 
> Thomas Weißschuh <linux@weissschuh.net>
> 
Re: [PATCH] ucounts: constify usage of ctl_table_header::ctl_table_arg
Posted by Joel Granados 1 year, 9 months ago
On Sun, May 12, 2024 at 09:22:13AM +0200, Thomas Weißschuh wrote:
> On 2024-05-11 21:28:10+0000, Thomas Weißschuh wrote:
> > The sysctl core is preparing to only expose instances of
> > struct ctl_table as "const".
> > This will also affect the member
> > ctl_table_header::ctl_table_arg.
> > 
> > Prepare for that change to "struct ctl_table_header",
> > and already constify the usage of ctl_table_arg.
> 
> Please disregard this patch.
> Apparently Dave already picked it up as part of [0] through the
> net-next tree, which makes sense the ctl_table_arg changes are in net/
> for the largest part.
> 
> See commit bfa858f220ab ("sysctl: treewide: constify ctl_table_header::ctl_table_arg").
Nice. Thx for pointing this out.

Best

-- 

Joel Granados