2024-05-11 19:28:30

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH] ucounts: constify usage of ctl_table_header::ctl_table_arg

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 <[email protected]>
---
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/[email protected]/
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/

Cc: Joel Granados <[email protected]>
Cc: Luis Chamberlain <[email protected]>
Cc: Eric W. Biederman <[email protected]>
---
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 <[email protected]>



2024-05-12 07:22:25

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH] ucounts: constify usage of ctl_table_header::ctl_table_arg

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/[email protected]/
>
> No functional change.
>
> Signed-off-by: Thomas Weißschuh <[email protected]>
> ---
> 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/[email protected]/
> [1] https://lore.kernel.org/lkml/[email protected]/
> [2] https://lore.kernel.org/lkml/[email protected]/
>
> Cc: Joel Granados <[email protected]>
> Cc: Luis Chamberlain <[email protected]>
> Cc: Eric W. Biederman <[email protected]>
> ---
> 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 <[email protected]>
>

2024-05-12 19:55:10

by Joel Granados

[permalink] [raw]
Subject: Re: [PATCH] ucounts: constify usage of ctl_table_header::ctl_table_arg

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


Attachments:
(No filename) (777.00 B)
signature.asc (673.00 B)
Download all attachments