Re: [PATCH v2 0/6] sysctl: prepare sysctl core for const struct ctl_table

[Joel Granados]

On Mon, Aug 05, 2024 at 11:39:34AM +0200, Thomas Weißschuh wrote:
> Adapt the internal and external APIs of the sysctl core to handle
> read-only instances of "struct ctl_table".

Finally getting around to this. Testing for this has been done on
sysctl-testing base:v6.11-rc6 and now on base:v6.12-rc2. Putting this in
sysctl-next so it will get further testing on its way to v6.13. First
patch (bugfix) will be ignored as it is already upstream.

Best

> 
> Patch 1: Bugfix for the sysctl core, the bug can be reliably triggered
>          with the series applied
> Patch 2: Trivial preparation commit for the sysctl BPF hook
> Patch 3: Adapts the internal sysctl APIs
> Patch 4: Adapts the external sysctl APIs
> Patch 5: Constifies the sysctl internal tables as proof that it works
> Patch 6: Updates scripts/const_structs.checkpatch for "struct ctl_table"
> 
> 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.
> 
> For this goal changes to both the sysctl core and "const" qualifiers for
> various sysctl APIs are necessary.
> 
> Full Process
> ============
> 
> * Drop ctl_table modifications from the sysctl core ([0], in mainline)
> * Constify arguments to ctl_table_root::{set_ownership,permissions}
>   ([1], in mainline)
> * Migrate users of "ctl_table_header::ctl_table_arg" to "const".
>   (in mainline)
> * Afterwards convert "ctl_table_header::ctl_table_arg" itself to const.
>   (in mainline)
> * Prepare helpers used to implement proc_handlers throughout the tree to
>   use "const struct ctl_table *". ([2], in mainline)
> * Afterwards switch over all proc_handlers callbacks to use
>   "const struct ctl_table *" in one commit. (in mainline)
> * Switch over the internals of the sysctl core to "const struct ctl_table *" (this series)
> * Switch include/linux/sysctl.h to "const struct ctl_table *" (this series)
> * Transition instances of "struct ctl_table" through the tree to const (to be done)
> 
> This series is meant to be applied through the sysctl tree.
> 
> Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
> ---
> Changes in v2:
> - Avoid spurious permanent empty tables (patch 1)
> - Link to v1: https://lore.kernel.org/r/20240729-sysctl-const-api-v1-0-ca628c7a942c@xxxxxxxxxxxxxx
> 
> ---
> Thomas Weißschuh (6):
>       sysctl: avoid spurious permanent empty tables
>       bpf: Constify ctl_table argument of filter function
>       sysctl: move internal interfaces to const struct ctl_table
>       sysctl: allow registration of const struct ctl_table
>       sysctl: make internal ctl_tables const
>       const_structs.checkpatch: add ctl_table
> 
>  fs/proc/internal.h               |   2 +-
>  fs/proc/proc_sysctl.c            | 100 +++++++++++++++++++++------------------
>  include/linux/bpf-cgroup.h       |   2 +-
>  include/linux/sysctl.h           |  12 ++---
>  kernel/bpf/cgroup.c              |   2 +-
>  scripts/const_structs.checkpatch |   1 +
>  6 files changed, 63 insertions(+), 56 deletions(-)
> ---
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
> change-id: 20240729-sysctl-const-api-73954f3d62c1
> 
> Best regards,
> -- 
> Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
> 

-- 

Joel Granados