Date: Sat, 25 Nov 2023 13:52:49 +0100
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Kees Cook <keescook@...omium.org>, 
 "Gustavo A. R. Silva" <gustavoars@...nel.org>, 
 Luis Chamberlain <mcgrof@...nel.org>, Iurii Zaikin <yzaikin@...gle.com>, 
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
 Joel Granados <j.granados@...sung.com>
Cc: linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-fsdevel@...r.kernel.org, 
 Thomas Weißschuh <linux@...ssschuh.net>
Subject: [PATCH RFC 0/7] sysctl: constify sysctl ctl_tables

Problem description:

The kernel contains a lot of struct ctl_table throught the tree.
These are very often 'static' definitions.
It would be good to mark these tables const to avoid accidental or
malicious modifications.
Unfortunately the tables can not be made const because the core
registration functions expect mutable tables.

This is for two reasons:

1) sysctl_{set,clear}_perm_empty_ctl_header in the sysctl core modify
   the table. This should be fixable by only modifying the header
   instead of the table itself.
2) The table is passed to the handler function as a non-const pointer.

This series is an aproach on fixing reason 2).

Full process:

* Introduce field proc_handler_new for const handlers (this series)
* Migrate all core handlers to proc_handler_new (this series, partial)
  This can hopefully be done in a big switch, as it only involves
  functions and structures owned by the core sysctl code.
* Migrate all other sysctl handlers to proc_handler_new.
* Drop the old proc_handler_field.
* Fix the sysctl core to not modify the tables anymore.
* Adapt public sysctl APIs to take "const struct ctl_table *".
* Teach checkpatch.pl to warn on non-const "struct ctl_table"
  definitions.
* Migrate definitions of "struct ctl_table" to "const" where applicable.
 

Notes:

Just casting the function pointers around would trigger
CFI (control flow integrity) warnings.

The name of the new handler "proc_handler_new" is a bit too long messing
up the alignment of the table definitions.
Maybe "proc_handler2" or "proc_handler_c" for (const) would be better.

---
Thomas Weißschuh (7):
      sysctl: add helper sysctl_run_handler
      bpf: cgroup: call proc handler through helper
      sysctl: add proc_handler_new to struct ctl_table
      net: sysctl: add new sysctl table handler to debug message
      treewide: sysctl: migrate proc_dostring to proc_handler_new
      treewide: sysctl: migrate proc_dobool to proc_handler_new
      treewide: sysctl: migrate proc_dointvec to proc_handler_new

 arch/arm/kernel/isa.c                   |  6 +--
 arch/csky/abiv1/alignment.c             |  8 ++--
 arch/powerpc/kernel/idle.c              |  2 +-
 arch/riscv/kernel/vector.c              |  2 +-
 arch/s390/kernel/debug.c                |  2 +-
 crypto/fips.c                           |  6 +--
 drivers/char/hpet.c                     |  2 +-
 drivers/char/random.c                   |  4 +-
 drivers/infiniband/core/iwcm.c          |  2 +-
 drivers/infiniband/core/ucma.c          |  2 +-
 drivers/macintosh/mac_hid.c             |  4 +-
 drivers/md/md.c                         |  4 +-
 drivers/scsi/sg.c                       |  2 +-
 drivers/tty/tty_io.c                    |  4 +-
 fs/coda/sysctl.c                        |  6 +--
 fs/coredump.c                           |  6 +--
 fs/devpts/inode.c                       |  2 +-
 fs/lockd/svc.c                          |  4 +-
 fs/locks.c                              |  4 +-
 fs/nfs/nfs4sysctl.c                     |  2 +-
 fs/nfs/sysctl.c                         |  2 +-
 fs/notify/dnotify/dnotify.c             |  2 +-
 fs/ntfs/sysctl.c                        |  2 +-
 fs/ocfs2/stackglue.c                    |  2 +-
 fs/proc/proc_sysctl.c                   | 16 ++++---
 fs/quota/dquot.c                        |  2 +-
 include/linux/sysctl.h                  | 29 +++++++++---
 init/do_mounts_initrd.c                 |  2 +-
 io_uring/io_uring.c                     |  2 +-
 ipc/mq_sysctl.c                         |  2 +-
 kernel/acct.c                           |  2 +-
 kernel/bpf/cgroup.c                     |  2 +-
 kernel/locking/lockdep.c                |  4 +-
 kernel/printk/sysctl.c                  |  4 +-
 kernel/reboot.c                         |  4 +-
 kernel/seccomp.c                        |  2 +-
 kernel/signal.c                         |  2 +-
 kernel/sysctl-test.c                    | 20 ++++-----
 kernel/sysctl.c                         | 80 ++++++++++++++++-----------------
 lib/test_sysctl.c                       | 10 ++---
 mm/hugetlb.c                            |  2 +-
 mm/hugetlb_vmemmap.c                    |  2 +-
 mm/oom_kill.c                           |  4 +-
 net/appletalk/sysctl_net_atalk.c        |  2 +-
 net/core/sysctl_net_core.c              | 12 ++---
 net/ipv4/route.c                        | 18 ++++----
 net/ipv4/sysctl_net_ipv4.c              | 38 ++++++++--------
 net/ipv4/xfrm4_policy.c                 |  2 +-
 net/ipv6/addrconf.c                     | 72 ++++++++++++++---------------
 net/ipv6/route.c                        |  8 ++--
 net/ipv6/sysctl_net_ipv6.c              | 18 ++++----
 net/ipv6/xfrm6_policy.c                 |  2 +-
 net/mptcp/ctrl.c                        |  2 +-
 net/netfilter/ipvs/ip_vs_ctl.c          | 36 +++++++--------
 net/netfilter/nf_conntrack_standalone.c |  8 ++--
 net/netfilter/nf_log.c                  |  2 +-
 net/rds/ib_sysctl.c                     |  2 +-
 net/rds/sysctl.c                        |  6 +--
 net/sctp/sysctl.c                       | 26 +++++------
 net/sunrpc/xprtrdma/transport.c         |  2 +-
 net/sysctl_net.c                        |  5 ++-
 net/unix/sysctl_net_unix.c              |  2 +-
 net/x25/sysctl_net_x25.c                |  2 +-
 net/xfrm/xfrm_sysctl.c                  |  4 +-
 64 files changed, 280 insertions(+), 262 deletions(-)
---
base-commit: 0f5cc96c367f2e780eb492cc9cab84e3b2ca88da
change-id: 20231116-const-sysctl-e14624f1295c

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

Powered by blists - more mailing lists