| Message ID | kndlh7lx2gfmz5m3ilwzp7fcsmimsnjgh434hnaro2pmy7evl6@jfui76m22kig (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL] sysctl constification changes for v6.14-rc1 | expand |
On Wed, 29 Jan 2025 at 00:14, Joel Granados <joel.granados@kernel.org> wrote: > > All ctl_table declared outside of functions and that remain unmodified after > initialization are const qualified. Hmm. A quick grep shows static struct ctl_table alignment_tbl[5] = { in arch/csky/abiv1/alignment.c that didn't get converted. And a couple of rdma drivers (iwcm_ctl_table and ucma_ctl_table), but maybe those weren't converted due to being in the "net" address space? Anyway, taken as-is, I'm just noting the lacking cases. Linus
The pull request you sent on Wed, 29 Jan 2025 09:14:20 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/sysctl/sysctl.git/ tags/constfy-sysctl-6.14-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/af13ff1c33e043b746cd96c83c7660ddf0272f73
Thank you!
On Wed, Jan 29, 2025 at 10:48:02AM -0800, Linus Torvalds wrote: > On Wed, 29 Jan 2025 at 00:14, Joel Granados <joel.granados@kernel.org> wrote: > > > > All ctl_table declared outside of functions and that remain unmodified after > > initialization are const qualified. > > Hmm. A quick grep shows > > static struct ctl_table alignment_tbl[5] = { Very good catch! I missed this one because it defines the size of the ctl_table array and my spatch did not account for that case. It turns out that the number in the square brackets does not coincide with the number of elements in the array. I would expect this results in a failure in the sysctl_check_table function; I dont have a csky system to test though. In any case there is an easy untested fix: From 431abf6c9c11a8b7321842ed0747b3200d43ef34 Mon Sep 17 00:00:00 2001 From: Joel Granados <joel.granados@kernel.org> Date: Fri, 31 Jan 2025 14:10:57 +0100 Subject: [PATCH] csky: Remove the size from alignment_tbl declaration Having to synchronize the number of ctl_table array elements with the size in the declaration can lead to discrepancies between the two values. Since commit d7a76ec87195 ("sysctl: Remove check for sentinel element in ctl_table arrays"), the calculation of the ctl_table array size is done solely by the ARRAY_SIZE macro removing the need for the size in the declaration. Remove the size for the aligment_tbl declaration and const qualify the array for good measure. Signed-off-by: Joel Granados <joel.granados@kernel.org> --- arch/csky/abiv1/alignment.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/csky/abiv1/alignment.c b/arch/csky/abiv1/alignment.c index e5b8b4b2109a..aee904833dec 100644 --- a/arch/csky/abiv1/alignment.c +++ b/arch/csky/abiv1/alignment.c @@ -300,7 +300,7 @@ void csky_alignment(struct pt_regs *regs) force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *)addr); } -static struct ctl_table alignment_tbl[5] = { +static const struct ctl_table alignment_tbl[] = { { .procname = "kernel_enable", .data = &align_kern_enable,
On Fri, Jan 31, 2025 at 02:57:40PM +0100, Joel Granados wrote: > From 431abf6c9c11a8b7321842ed0747b3200d43ef34 Mon Sep 17 00:00:00 2001 > From: Joel Granados <joel.granados@kernel.org> > Date: Fri, 31 Jan 2025 14:10:57 +0100 > Subject: [PATCH] csky: Remove the size from alignment_tbl declaration > > Having to synchronize the number of ctl_table array elements with the > size in the declaration can lead to discrepancies between the two > values. Since commit d7a76ec87195 ("sysctl: Remove check for sentinel > element in ctl_table arrays"), the calculation of the ctl_table array > size is done solely by the ARRAY_SIZE macro removing the need for the > size in the declaration. > > Remove the size for the aligment_tbl declaration and const qualify the > array for good measure. > > Signed-off-by: Joel Granados <joel.granados@kernel.org> FWIW, this looks correct to me. Reviewed-by: Kees Cook <kees@kernel.org>
Linus: This is a continuation of the ctl_table constification work started in commit 78eb4ea25cd5 ("sysctl: treewide: constify the ctl_table argument of proc_handlers"). That commit allows the const qualifying of the ctl_tables; with a few exceptions listed in the commit. As with previous large tree-wide PRs [1], I'm sending it at the end of the merge window to try to avoid unnecessary conflicts. I have rebased it on top of what I see as your latest master (6d61a53dd6f5), but I can rebase it again later if you prefer. Testing was done in 0-day to avoid generating unnecessary merge conflicts in linux-next. I do not expect any error/regression given that all changes contained in this PR are non-functional. Finally, if you need to regenerate it do: 1. Run the spatch [2] with the coccichekck command [3]. 2. Run the sed command [4] Best [1] https://lore.kernel.org/all/20240724210014.mc6nima6cekgiukx@joelS2.panther.com/ [2] Spatch: virtual patch @ depends on !(file in "net") disable optional_qualifier @ identifier table_name != { watchdog_hardlockup_sysctl, iwcm_ctl_table, ucma_ctl_table, memory_allocation_profiling_sysctls, loadpin_sysctl_table }; @@ + const struct ctl_table table_name [] = { ... }; [3] Spatch command: make coccicheck MODE=patch \ SPFLAGS="--in-place --include-headers --smpl-spacing --jobs=16" \ COCCI=PATCH_FILE [4] sed: sed --in-place \ -e "s/struct ctl_table .table = &uts_kern/const struct ctl_table *table = \&uts_kern/" \ kernel/utsname_sysctl.c The following changes since commit 6d61a53dd6f55405ebcaea6ee38d1ab5a8856c2c: Merge tag 'f2fs-for-6.14-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs (2025-01-27 20:58:58 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sysctl/sysctl.git/ tags/constfy-sysctl-6.14-rc1 for you to fetch changes up to 1751f872cc97f992ed5c4c72c55588db1f0021e1: treewide: const qualify ctl_tables where applicable (2025-01-28 13:48:37 +0100) ---------------------------------------------------------------- Summary: All ctl_table declared outside of functions and that remain unmodified after initialization are const qualified. This prevents unintended modifications to proc_handler function pointers by placing them in the .rodata section. This is a continuation of the tree-wide effort started a few releases ago with the constification of the ctl_table struct arguments in the sysctl API done in 78eb4ea25cd5 ("sysctl: treewide: constify the ctl_table argument of proc_handlers") Testing: Testing was done on 0-day and sysctl selftests in x86_64. The linux-next branch was not used for such a big change in order to avoid unnecessary merge conflicts ---------------------------------------------------------------- Joel Granados (1): treewide: const qualify ctl_tables where applicable arch/arm/kernel/isa.c | 2 +- arch/arm64/kernel/fpsimd.c | 4 ++-- arch/arm64/kernel/process.c | 2 +- arch/powerpc/kernel/idle.c | 2 +- arch/powerpc/platforms/pseries/mobility.c | 2 +- arch/riscv/kernel/process.c | 2 +- arch/riscv/kernel/vector.c | 2 +- arch/s390/appldata/appldata_base.c | 2 +- arch/s390/kernel/debug.c | 2 +- arch/s390/kernel/hiperdispatch.c | 2 +- arch/s390/kernel/topology.c | 2 +- arch/s390/mm/cmm.c | 2 +- arch/s390/mm/pgalloc.c | 2 +- arch/x86/entry/vdso/vdso32-setup.c | 2 +- arch/x86/kernel/cpu/bus_lock.c | 2 +- crypto/fips.c | 2 +- drivers/base/firmware_loader/fallback_table.c | 2 +- drivers/cdrom/cdrom.c | 2 +- drivers/char/hpet.c | 2 +- drivers/char/ipmi/ipmi_poweroff.c | 2 +- drivers/char/random.c | 2 +- drivers/gpu/drm/i915/i915_perf.c | 2 +- drivers/gpu/drm/xe/xe_observation.c | 2 +- drivers/hv/hv_common.c | 2 +- drivers/md/md.c | 2 +- drivers/misc/sgi-xp/xpc_main.c | 4 ++-- drivers/perf/arm_pmuv3.c | 2 +- drivers/perf/riscv_pmu_sbi.c | 2 +- drivers/scsi/scsi_sysctl.c | 2 +- drivers/scsi/sg.c | 2 +- drivers/tty/tty_io.c | 2 +- drivers/xen/balloon.c | 2 +- fs/aio.c | 2 +- fs/cachefiles/error_inject.c | 2 +- fs/coda/sysctl.c | 2 +- fs/coredump.c | 2 +- fs/dcache.c | 2 +- fs/devpts/inode.c | 2 +- fs/eventpoll.c | 2 +- fs/exec.c | 2 +- fs/file_table.c | 2 +- fs/fuse/sysctl.c | 2 +- fs/inode.c | 2 +- fs/lockd/svc.c | 2 +- fs/locks.c | 2 +- fs/namei.c | 2 +- fs/namespace.c | 2 +- fs/nfs/nfs4sysctl.c | 2 +- fs/nfs/sysctl.c | 2 +- fs/notify/dnotify/dnotify.c | 2 +- fs/notify/fanotify/fanotify_user.c | 2 +- fs/notify/inotify/inotify_user.c | 2 +- fs/ocfs2/stackglue.c | 2 +- fs/pipe.c | 2 +- fs/quota/dquot.c | 2 +- fs/sysctls.c | 2 +- fs/userfaultfd.c | 2 +- fs/verity/init.c | 2 +- fs/xfs/xfs_sysctl.c | 2 +- init/do_mounts_initrd.c | 2 +- io_uring/io_uring.c | 2 +- ipc/ipc_sysctl.c | 2 +- ipc/mq_sysctl.c | 2 +- kernel/acct.c | 2 +- kernel/bpf/syscall.c | 2 +- kernel/delayacct.c | 2 +- kernel/exit.c | 2 +- kernel/hung_task.c | 2 +- kernel/kexec_core.c | 2 +- kernel/kprobes.c | 2 +- kernel/latencytop.c | 2 +- kernel/locking/lockdep.c | 2 +- kernel/panic.c | 2 +- kernel/pid.c | 2 +- kernel/pid_namespace.c | 2 +- kernel/pid_sysctl.h | 2 +- kernel/printk/sysctl.c | 2 +- kernel/reboot.c | 2 +- kernel/sched/autogroup.c | 2 +- kernel/sched/core.c | 2 +- kernel/sched/deadline.c | 2 +- kernel/sched/fair.c | 2 +- kernel/sched/rt.c | 2 +- kernel/sched/topology.c | 2 +- kernel/seccomp.c | 2 +- kernel/signal.c | 2 +- kernel/stackleak.c | 2 +- kernel/sysctl-test.c | 6 +++--- kernel/sysctl.c | 4 ++-- kernel/time/timer.c | 2 +- kernel/trace/ftrace.c | 2 +- kernel/trace/trace_events_user.c | 2 +- kernel/umh.c | 2 +- kernel/utsname_sysctl.c | 4 ++-- kernel/watchdog.c | 2 +- lib/test_sysctl.c | 6 +++--- mm/compaction.c | 2 +- mm/hugetlb.c | 2 +- mm/hugetlb_vmemmap.c | 2 +- mm/memory-failure.c | 2 +- mm/oom_kill.c | 2 +- mm/page-writeback.c | 2 +- mm/page_alloc.c | 2 +- security/apparmor/lsm.c | 2 +- security/keys/sysctl.c | 2 +- security/yama/yama_lsm.c | 2 +- 106 files changed, 114 insertions(+), 114 deletions(-)