From c09b9810414d09d5a66e6e397506f5c657303eef Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Sun, 22 Dec 2024 22:54:15 +0100 Subject: [PATCH 1/9] panic: Move panic ctl tables into panic.c Move panic, panic_on_oops, panic_print, panic_on_warn into kerne/panic.c. This is part of a greater effort to move ctl tables into their respective subsystems which will reduce the merge conflicts in kernel/sysctl.c. Signed-off-by: Joel Granados --- kernel/panic.c | 30 ++++++++++++++++++++++++++++++ kernel/sysctl.c | 31 ------------------------------- 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/kernel/panic.c b/kernel/panic.c index a3889f38153d..047ea3215312 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -96,6 +96,36 @@ static const struct ctl_table kern_panic_table[] = { .extra2 = SYSCTL_ONE, }, #endif + { + .procname = "panic", + .data = &panic_timeout, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, + { + .procname = "panic_on_oops", + .data = &panic_on_oops, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, + { + .procname = "panic_print", + .data = &panic_print, + .maxlen = sizeof(unsigned long), + .mode = 0644, + .proc_handler = proc_doulongvec_minmax, + }, + { + .procname = "panic_on_warn", + .data = &panic_on_warn, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec_minmax, + .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_ONE, + }, { .procname = "warn_limit", .data = &warn_limit, diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 3b7a7308e35b..7c12bbdca51a 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include @@ -1588,13 +1587,6 @@ int proc_do_static_key(const struct ctl_table *table, int write, } static const struct ctl_table kern_table[] = { - { - .procname = "panic", - .data = &panic_timeout, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - }, #ifdef CONFIG_PROC_SYSCTL { .procname = "tainted", @@ -1772,20 +1764,6 @@ static const struct ctl_table kern_table[] = { .extra1 = SYSCTL_ZERO, .extra2 = SYSCTL_MAXOLDUID, }, - { - .procname = "panic_on_oops", - .data = &panic_on_oops, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - }, - { - .procname = "panic_print", - .data = &panic_print, - .maxlen = sizeof(unsigned long), - .mode = 0644, - .proc_handler = proc_doulongvec_minmax, - }, { .procname = "ngroups_max", .data = (void *)&ngroups_max, @@ -1837,15 +1815,6 @@ static const struct ctl_table kern_table[] = { .proc_handler = proc_dointvec, }, #endif - { - .procname = "panic_on_warn", - .data = &panic_on_warn, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE, - }, #ifdef CONFIG_TREE_RCU { .procname = "panic_on_rcu_stall", From 256db5c9b507d770f8b99b10f0671d994d5c9d64 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Mon, 23 Dec 2024 17:11:29 +0100 Subject: [PATCH 2/9] signal: Move signal ctl tables into signal.c Move print-fatal-signals into its own const ctl table array in kernel/signal.c. This is part of a greater effort to move ctl tables into their respective subsystems which will reduce the merge conflicts in kernel/sysctl.c. Signed-off-by: Joel Granados --- kernel/signal.c | 11 +++++++++++ kernel/sysctl.c | 8 -------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/kernel/signal.c b/kernel/signal.c index f8859faa26c5..148082db9a55 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -4981,9 +4981,20 @@ static const struct ctl_table signal_debug_table[] = { #endif }; +static const struct ctl_table signal_table[] = { + { + .procname = "print-fatal-signals", + .data = &print_fatal_signals, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, +}; + static int __init init_signal_sysctls(void) { register_sysctl_init("debug", signal_debug_table); + register_sysctl_init("kernel", signal_table); return 0; } early_initcall(init_signal_sysctls); diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 7c12bbdca51a..811c50072e03 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -22,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -1604,13 +1603,6 @@ static const struct ctl_table kern_table[] = { .extra2 = SYSCTL_ONE, }, #endif - { - .procname = "print-fatal-signals", - .data = &print_fatal_signals, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - }, #ifdef CONFIG_SPARC { .procname = "reboot-cmd", From dd293df6395a2c9e0fc4faa8defeceaa907e7717 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Sun, 29 Dec 2024 16:11:25 +0100 Subject: [PATCH 3/9] tracing: Move trace sysctls into trace.c Move trace ctl tables into their own const array in kernel/trace/trace.c. The sysctl table register is called with subsys_initcall placing if after its original place in proc_root_init. This is part of a greater effort to move ctl tables into their respective subsystems which will reduce the merge conflicts in kernel/sysctl.c. Signed-off-by: Joel Granados Acked-by: Steven Rostedt (Google) --- include/linux/ftrace.h | 7 ------- kernel/sysctl.c | 24 ------------------------ kernel/trace/trace.c | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 35 insertions(+), 32 deletions(-) diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index fbabc3d848b3..59774513ae45 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -1298,16 +1298,9 @@ static inline void unpause_graph_tracing(void) { } #ifdef CONFIG_TRACING enum ftrace_dump_mode; -#define MAX_TRACER_SIZE 100 -extern char ftrace_dump_on_oops[]; extern int ftrace_dump_on_oops_enabled(void); -extern int tracepoint_printk; extern void disable_trace_on_warning(void); -extern int __disable_trace_on_warning; - -int tracepoint_printk_sysctl(const struct ctl_table *table, int write, - void *buffer, size_t *lenp, loff_t *ppos); #else /* CONFIG_TRACING */ static inline void disable_trace_on_warning(void) { } diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 811c50072e03..324ae43a3d50 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -44,7 +44,6 @@ #include #include #include -#include #include #include #include @@ -1662,29 +1661,6 @@ static const struct ctl_table kern_table[] = { .proc_handler = stack_trace_sysctl, }, #endif -#ifdef CONFIG_TRACING - { - .procname = "ftrace_dump_on_oops", - .data = &ftrace_dump_on_oops, - .maxlen = MAX_TRACER_SIZE, - .mode = 0644, - .proc_handler = proc_dostring, - }, - { - .procname = "traceoff_on_warning", - .data = &__disable_trace_on_warning, - .maxlen = sizeof(__disable_trace_on_warning), - .mode = 0644, - .proc_handler = proc_dointvec, - }, - { - .procname = "tracepoint_printk", - .data = &tracepoint_printk, - .maxlen = sizeof(tracepoint_printk), - .mode = 0644, - .proc_handler = tracepoint_printk_sysctl, - }, -#endif #ifdef CONFIG_MODULES { .procname = "modprobe", diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index b581e388a9d9..bd9c30dd1183 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -120,6 +120,7 @@ static int tracing_disabled = 1; cpumask_var_t __read_mostly tracing_buffer_mask; +#define MAX_TRACER_SIZE 100 /* * ftrace_dump_on_oops - variable to dump ftrace buffer on oops * @@ -142,7 +143,40 @@ cpumask_var_t __read_mostly tracing_buffer_mask; char ftrace_dump_on_oops[MAX_TRACER_SIZE] = "0"; /* When set, tracing will stop when a WARN*() is hit */ -int __disable_trace_on_warning; +static int __disable_trace_on_warning; + +int tracepoint_printk_sysctl(const struct ctl_table *table, int write, + void *buffer, size_t *lenp, loff_t *ppos); +static const struct ctl_table trace_sysctl_table[] = { + { + .procname = "ftrace_dump_on_oops", + .data = &ftrace_dump_on_oops, + .maxlen = MAX_TRACER_SIZE, + .mode = 0644, + .proc_handler = proc_dostring, + }, + { + .procname = "traceoff_on_warning", + .data = &__disable_trace_on_warning, + .maxlen = sizeof(__disable_trace_on_warning), + .mode = 0644, + .proc_handler = proc_dointvec, + }, + { + .procname = "tracepoint_printk", + .data = &tracepoint_printk, + .maxlen = sizeof(tracepoint_printk), + .mode = 0644, + .proc_handler = tracepoint_printk_sysctl, + }, +}; + +static int __init init_trace_sysctls(void) +{ + register_sysctl_init("kernel", trace_sysctl_table); + return 0; +} +subsys_initcall(init_trace_sysctls); #ifdef CONFIG_TRACE_EVAL_MAP_FILE /* Map of enums to their values, for "eval_map" file */ From 67049b53e06fa1758df1463789f286a7cba67c50 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Wed, 8 Jan 2025 12:55:58 +0100 Subject: [PATCH 4/9] stack_tracer: move sysctl registration to kernel/trace/trace_stack.c Move stack_tracer_enabled into trace_stack_sysctl_table. This is part of a greater effort to move ctl tables into their respective subsystems which will reduce the merge conflicts in kernel/sysctl.c. Acked-by: Steven Rostedt (Google) Signed-off-by: Joel Granados --- include/linux/ftrace.h | 2 -- kernel/sysctl.c | 10 ---------- kernel/trace/trace_stack.c | 22 +++++++++++++++++++++- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 59774513ae45..95851a6fb942 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -569,8 +569,6 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, #ifdef CONFIG_STACK_TRACER -extern int stack_tracer_enabled; - int stack_trace_sysctl(const struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos); diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 324ae43a3d50..e5430594dfd6 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -58,7 +58,6 @@ #ifdef CONFIG_X86 #include -#include #include #endif #ifdef CONFIG_SPARC @@ -1652,15 +1651,6 @@ static const struct ctl_table kern_table[] = { .proc_handler = proc_dointvec, }, #endif -#ifdef CONFIG_STACK_TRACER - { - .procname = "stack_tracer_enabled", - .data = &stack_tracer_enabled, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = stack_trace_sysctl, - }, -#endif #ifdef CONFIG_MODULES { .procname = "modprobe", diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c index 14c6f272c4d8..e34223c8065d 100644 --- a/kernel/trace/trace_stack.c +++ b/kernel/trace/trace_stack.c @@ -32,7 +32,7 @@ static arch_spinlock_t stack_trace_max_lock = DEFINE_PER_CPU(int, disable_stack_tracer); static DEFINE_MUTEX(stack_sysctl_mutex); -int stack_tracer_enabled; +static int stack_tracer_enabled; static void print_max_stack(void) { @@ -578,3 +578,23 @@ static __init int stack_trace_init(void) } device_initcall(stack_trace_init); + + +static const struct ctl_table trace_stack_sysctl_table[] = { + { + .procname = "stack_tracer_enabled", + .data = &stack_tracer_enabled, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = stack_trace_sysctl, + }, +}; + +static int __init init_trace_stack_sysctls(void) +{ + register_sysctl_init("kernel", trace_stack_sysctl_table); + return 0; +} +subsys_initcall(init_trace_stack_sysctls); + + From bc4f328ff516f6ea53c3c0d385a84ea7ae423c20 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Tue, 7 Jan 2025 15:19:19 +0100 Subject: [PATCH 5/9] sparc: mv sparc sysctls into their own file under arch/sparc/kernel Move sparc sysctls (reboot-cmd, stop-a, scons-poweroff and tsb-ratio) into a new file (arch/sparc/kernel/setup.c). This file will be included for both 32 and 64 bit sparc. Leave "tsb-ratio" under SPARC64 ifdef as it was in kernel/sysctl.c. The sysctl table register is called with arch_initcall placing it after its original place in proc_root_init. This is part of a greater effort to move ctl tables into their respective subsystems which will reduce the merge conflicts in kernel/sysctl.c. Signed-off-by: Joel Granados --- arch/sparc/kernel/Makefile | 1 + arch/sparc/kernel/setup.c | 46 ++++++++++++++++++++++++++++++++++++++ kernel/sysctl.c | 35 ----------------------------- 3 files changed, 47 insertions(+), 35 deletions(-) create mode 100644 arch/sparc/kernel/setup.c diff --git a/arch/sparc/kernel/Makefile b/arch/sparc/kernel/Makefile index 58ea4ef9b622..3453f330e363 100644 --- a/arch/sparc/kernel/Makefile +++ b/arch/sparc/kernel/Makefile @@ -35,6 +35,7 @@ obj-y += process.o obj-y += signal_$(BITS).o obj-y += sigutil_$(BITS).o obj-$(CONFIG_SPARC32) += ioport.o +obj-y += setup.o obj-y += setup_$(BITS).o obj-y += idprom.o obj-y += sys_sparc_$(BITS).o diff --git a/arch/sparc/kernel/setup.c b/arch/sparc/kernel/setup.c new file mode 100644 index 000000000000..4975867d9001 --- /dev/null +++ b/arch/sparc/kernel/setup.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include + +static const struct ctl_table sparc_sysctl_table[] = { + { + .procname = "reboot-cmd", + .data = reboot_command, + .maxlen = 256, + .mode = 0644, + .proc_handler = proc_dostring, + }, + { + .procname = "stop-a", + .data = &stop_a_enabled, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, + { + .procname = "scons-poweroff", + .data = &scons_pwroff, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, +#ifdef CONFIG_SPARC64 + { + .procname = "tsb-ratio", + .data = &sysctl_tsb_ratio, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, +#endif +}; + + +static int __init init_sparc_sysctls(void) +{ + register_sysctl_init("kernel", sparc_sysctl_table); + return 0; +} + +arch_initcall(init_sparc_sysctls); diff --git a/kernel/sysctl.c b/kernel/sysctl.c index e5430594dfd6..9b4f0cff76ea 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -60,9 +60,6 @@ #include #include #endif -#ifdef CONFIG_SPARC -#include -#endif #ifdef CONFIG_RT_MUTEXES #include #endif @@ -1601,38 +1598,6 @@ static const struct ctl_table kern_table[] = { .extra2 = SYSCTL_ONE, }, #endif -#ifdef CONFIG_SPARC - { - .procname = "reboot-cmd", - .data = reboot_command, - .maxlen = 256, - .mode = 0644, - .proc_handler = proc_dostring, - }, - { - .procname = "stop-a", - .data = &stop_a_enabled, - .maxlen = sizeof (int), - .mode = 0644, - .proc_handler = proc_dointvec, - }, - { - .procname = "scons-poweroff", - .data = &scons_pwroff, - .maxlen = sizeof (int), - .mode = 0644, - .proc_handler = proc_dointvec, - }, -#endif -#ifdef CONFIG_SPARC64 - { - .procname = "tsb-ratio", - .data = &sysctl_tsb_ratio, - .maxlen = sizeof (int), - .mode = 0644, - .proc_handler = proc_dointvec, - }, -#endif #ifdef CONFIG_PARISC { .procname = "soft-power", From 138303ec6ccbe38611931eeb955a722c6f78ec25 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Thu, 13 Mar 2025 22:35:25 +0100 Subject: [PATCH 6/9] sysctl: move u8 register test to lib/test_sysctl.c If the test added in commit b5ffbd139688 ("sysctl: move the extra1/2 boundary check of u8 to sysctl_check_table_array") is run as a module, a lingering reference to the module is left behind, and a 'sysctl -a' leads to a panic. To reproduce CONFIG_KUNIT=y CONFIG_SYSCTL_KUNIT_TEST=m Then run these commands: modprobe sysctl-test rmmod sysctl-test sysctl -a The panic varies but generally looks something like this: BUG: unable to handle page fault for address: ffffa4571c0c7db4 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 100000067 P4D 100000067 PUD 100351067 PMD 114f5e067 PTE 0 Oops: Oops: 0000 [#1] SMP NOPTI ... ... ... RIP: 0010:proc_sys_readdir+0x166/0x2c0 ... ... ... Call Trace: iterate_dir+0x6e/0x140 __se_sys_getdents+0x6e/0x100 do_syscall_64+0x70/0x150 entry_SYSCALL_64_after_hwframe+0x76/0x7e Move the test to lib/test_sysctl.c where the registration reference is handled on module exit Fixes: b5ffbd139688 ("sysctl: move the extra1/2 boundary check of u8 to sysctl_check_table_array") Reviewed-by: Kees Cook Signed-off-by: Joel Granados --- kernel/sysctl-test.c | 49 -------------------------------- lib/test_sysctl.c | 66 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 49 deletions(-) diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c index eb2842bd0557..92f94ea28957 100644 --- a/kernel/sysctl-test.c +++ b/kernel/sysctl-test.c @@ -367,54 +367,6 @@ static void sysctl_test_api_dointvec_write_single_greater_int_max( KUNIT_EXPECT_EQ(test, 0, *((int *)table.data)); } -/* - * Test that registering an invalid extra value is not allowed. - */ -static void sysctl_test_register_sysctl_sz_invalid_extra_value( - struct kunit *test) -{ - unsigned char data = 0; - const struct ctl_table table_foo[] = { - { - .procname = "foo", - .data = &data, - .maxlen = sizeof(u8), - .mode = 0644, - .proc_handler = proc_dou8vec_minmax, - .extra1 = SYSCTL_FOUR, - .extra2 = SYSCTL_ONE_THOUSAND, - }, - }; - - const struct ctl_table table_bar[] = { - { - .procname = "bar", - .data = &data, - .maxlen = sizeof(u8), - .mode = 0644, - .proc_handler = proc_dou8vec_minmax, - .extra1 = SYSCTL_NEG_ONE, - .extra2 = SYSCTL_ONE_HUNDRED, - }, - }; - - const struct ctl_table table_qux[] = { - { - .procname = "qux", - .data = &data, - .maxlen = sizeof(u8), - .mode = 0644, - .proc_handler = proc_dou8vec_minmax, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_TWO_HUNDRED, - }, - }; - - KUNIT_EXPECT_NULL(test, register_sysctl("foo", table_foo)); - KUNIT_EXPECT_NULL(test, register_sysctl("foo", table_bar)); - KUNIT_EXPECT_NOT_NULL(test, register_sysctl("foo", table_qux)); -} - static struct kunit_case sysctl_test_cases[] = { KUNIT_CASE(sysctl_test_api_dointvec_null_tbl_data), KUNIT_CASE(sysctl_test_api_dointvec_table_maxlen_unset), @@ -426,7 +378,6 @@ static struct kunit_case sysctl_test_cases[] = { KUNIT_CASE(sysctl_test_dointvec_write_happy_single_negative), KUNIT_CASE(sysctl_test_api_dointvec_write_single_less_int_min), KUNIT_CASE(sysctl_test_api_dointvec_write_single_greater_int_max), - KUNIT_CASE(sysctl_test_register_sysctl_sz_invalid_extra_value), {} }; diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c index 4249e0cc8aaf..54a22e4b1346 100644 --- a/lib/test_sysctl.c +++ b/lib/test_sysctl.c @@ -37,6 +37,7 @@ static struct { struct ctl_table_header *test_h_mnterror; struct ctl_table_header *empty_add; struct ctl_table_header *empty; + struct ctl_table_header *test_u8; } sysctl_test_headers; struct test_sysctl_data { @@ -239,6 +240,65 @@ static int test_sysctl_run_register_empty(void) return 0; } +static const struct ctl_table table_u8_over[] = { + { + .procname = "u8_over", + .data = &test_data.uint_0001, + .maxlen = sizeof(u8), + .mode = 0644, + .proc_handler = proc_dou8vec_minmax, + .extra1 = SYSCTL_FOUR, + .extra2 = SYSCTL_ONE_THOUSAND, + }, +}; + +static const struct ctl_table table_u8_under[] = { + { + .procname = "u8_under", + .data = &test_data.uint_0001, + .maxlen = sizeof(u8), + .mode = 0644, + .proc_handler = proc_dou8vec_minmax, + .extra1 = SYSCTL_NEG_ONE, + .extra2 = SYSCTL_ONE_HUNDRED, + }, +}; + +static const struct ctl_table table_u8_valid[] = { + { + .procname = "u8_valid", + .data = &test_data.uint_0001, + .maxlen = sizeof(u8), + .mode = 0644, + .proc_handler = proc_dou8vec_minmax, + .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_TWO_HUNDRED, + }, +}; + +static int test_sysctl_register_u8_extra(void) +{ + /* should fail because it's over */ + sysctl_test_headers.test_u8 + = register_sysctl("debug/test_sysctl", table_u8_over); + if (sysctl_test_headers.test_u8) + return -ENOMEM; + + /* should fail because it's under */ + sysctl_test_headers.test_u8 + = register_sysctl("debug/test_sysctl", table_u8_under); + if (sysctl_test_headers.test_u8) + return -ENOMEM; + + /* should not fail because it's valid */ + sysctl_test_headers.test_u8 + = register_sysctl("debug/test_sysctl", table_u8_valid); + if (!sysctl_test_headers.test_u8) + return -ENOMEM; + + return 0; +} + static int __init test_sysctl_init(void) { int err; @@ -256,6 +316,10 @@ static int __init test_sysctl_init(void) goto out; err = test_sysctl_run_register_empty(); + if (err) + goto out; + + err = test_sysctl_register_u8_extra(); out: return err; @@ -275,6 +339,8 @@ static void __exit test_sysctl_exit(void) unregister_sysctl_table(sysctl_test_headers.empty); if (sysctl_test_headers.empty_add) unregister_sysctl_table(sysctl_test_headers.empty_add); + if (sysctl_test_headers.test_u8) + unregister_sysctl_table(sysctl_test_headers.test_u8); } module_exit(test_sysctl_exit); From 8e4acabdc8691529c163c18a45bcdd332bd75145 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Fri, 21 Mar 2025 09:44:39 +0100 Subject: [PATCH 7/9] sysctl: Add 0012 to test the u8 range check Add a sysctl test that uses the new u8 test ctl files in a created by the sysctl test module. Check that the u8 proc file that is valid is created and that there are two messages in dmesg for the files that were out of range. Reviewed-by: Kees Cook Signed-off-by: Joel Granados --- tools/testing/selftests/sysctl/sysctl.sh | 30 ++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh index db1616857d89..a10350c8a46e 100755 --- a/tools/testing/selftests/sysctl/sysctl.sh +++ b/tools/testing/selftests/sysctl/sysctl.sh @@ -36,6 +36,7 @@ ALL_TESTS="$ALL_TESTS 0008:1:1:match_int:1" ALL_TESTS="$ALL_TESTS 0009:1:1:unregister_error:0" ALL_TESTS="$ALL_TESTS 0010:1:1:mnt/mnt_error:0" ALL_TESTS="$ALL_TESTS 0011:1:1:empty_add:0" +ALL_TESTS="$ALL_TESTS 0012:1:1:u8_valid:0" function allow_user_defaults() { @@ -851,6 +852,34 @@ sysctl_test_0011() return 0 } +sysctl_test_0012() +{ + TARGET="${SYSCTL}/$(get_test_target 0012)" + echo -n "Testing u8 range check in sysctl table check in ${TARGET} ... " + if [ ! -f ${TARGET} ]; then + echo -e "FAIL\nCould not create ${TARGET}" >&2 + rc=1 + test_rc + fi + + local u8over_msg=$(dmesg | grep "u8_over range value" | wc -l) + if [ ! ${u8over_msg} -eq 1 ]; then + echo -e "FAIL\nu8 overflow not detected" >&2 + rc=1 + test_rc + fi + + local u8under_msg=$(dmesg | grep "u8_under range value" | wc -l) + if [ ! ${u8under_msg} -eq 1 ]; then + echo -e "FAIL\nu8 underflow not detected" >&2 + rc=1 + test_rc + fi + + echo "OK" + return 0 +} + list_tests() { echo "Test ID list:" @@ -870,6 +899,7 @@ list_tests() echo "0009 x $(get_test_count 0009) - tests sysct unregister" echo "0010 x $(get_test_count 0010) - tests sysct mount point" echo "0011 x $(get_test_count 0011) - tests empty directories" + echo "0012 x $(get_test_count 0012) - tests range check for u8 proc_handler" } usage() From 2bac112eaaf391f190905134cc8e7ffc02dd131c Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Tue, 18 Mar 2025 22:04:28 +0100 Subject: [PATCH 8/9] sysctl: call sysctl tests with a for loop As we add more test functions in lib/tests_sysctl the main test function (test_sysctl_init) grows. Condense the logic to make it easier to add/remove tests. Reviewed-by: Kees Cook Signed-off-by: Joel Granados --- lib/test_sysctl.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c index 54a22e4b1346..4b3d56de6269 100644 --- a/lib/test_sysctl.c +++ b/lib/test_sysctl.c @@ -301,27 +301,19 @@ static int test_sysctl_register_u8_extra(void) static int __init test_sysctl_init(void) { - int err; + int err = 0; - err = test_sysctl_setup_node_tests(); - if (err) - goto out; + int (*func_array[])(void) = { + test_sysctl_setup_node_tests, + test_sysctl_run_unregister_nested, + test_sysctl_run_register_mount_point, + test_sysctl_run_register_empty, + test_sysctl_register_u8_extra + }; - err = test_sysctl_run_unregister_nested(); - if (err) - goto out; + for (int i = 0; !err && i < ARRAY_SIZE(func_array); i++) + err = func_array[i](); - err = test_sysctl_run_register_mount_point(); - if (err) - goto out; - - err = test_sysctl_run_register_empty(); - if (err) - goto out; - - err = test_sysctl_register_u8_extra(); - -out: return err; } module_init(test_sysctl_init); From 23b8bacf154759ed922d25527dda434fbf57436a Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Tue, 18 Mar 2025 22:30:09 +0100 Subject: [PATCH 9/9] sysctl: Close test ctl_headers with a for loop As more tests are added, the exit function gets longer than it should be. Condense the un-register calls into a for loop to make it easier to add/remove tests. Reviewed-by: Kees Cook Signed-off-by: Joel Granados --- lib/test_sysctl.c | 65 +++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c index 4b3d56de6269..c02aa9c868f2 100644 --- a/lib/test_sysctl.c +++ b/lib/test_sysctl.c @@ -30,16 +30,17 @@ static int i_zero; static int i_one_hundred = 100; static int match_int_ok = 1; +enum { + TEST_H_SETUP_NODE, + TEST_H_MNT, + TEST_H_MNTERROR, + TEST_H_EMPTY_ADD, + TEST_H_EMPTY, + TEST_H_U8, + TEST_H_SIZE /* Always at the end */ +}; -static struct { - struct ctl_table_header *test_h_setup_node; - struct ctl_table_header *test_h_mnt; - struct ctl_table_header *test_h_mnterror; - struct ctl_table_header *empty_add; - struct ctl_table_header *empty; - struct ctl_table_header *test_u8; -} sysctl_test_headers; - +static struct ctl_table_header *ctl_headers[TEST_H_SIZE] = {}; struct test_sysctl_data { int int_0001; int int_0002; @@ -168,8 +169,8 @@ static int test_sysctl_setup_node_tests(void) test_data.bitmap_0001 = kzalloc(SYSCTL_TEST_BITMAP_SIZE/8, GFP_KERNEL); if (!test_data.bitmap_0001) return -ENOMEM; - sysctl_test_headers.test_h_setup_node = register_sysctl("debug/test_sysctl", test_table); - if (!sysctl_test_headers.test_h_setup_node) { + ctl_headers[TEST_H_SETUP_NODE] = register_sysctl("debug/test_sysctl", test_table); + if (!ctl_headers[TEST_H_SETUP_NODE]) { kfree(test_data.bitmap_0001); return -ENOMEM; } @@ -203,12 +204,12 @@ static int test_sysctl_run_unregister_nested(void) static int test_sysctl_run_register_mount_point(void) { - sysctl_test_headers.test_h_mnt + ctl_headers[TEST_H_MNT] = register_sysctl_mount_point("debug/test_sysctl/mnt"); - if (!sysctl_test_headers.test_h_mnt) + if (!ctl_headers[TEST_H_MNT]) return -ENOMEM; - sysctl_test_headers.test_h_mnterror + ctl_headers[TEST_H_MNTERROR] = register_sysctl("debug/test_sysctl/mnt/mnt_error", test_table_unregister); /* @@ -226,15 +227,15 @@ static const struct ctl_table test_table_empty[] = { }; static int test_sysctl_run_register_empty(void) { /* Tets that an empty dir can be created */ - sysctl_test_headers.empty_add + ctl_headers[TEST_H_EMPTY_ADD] = register_sysctl("debug/test_sysctl/empty_add", test_table_empty); - if (!sysctl_test_headers.empty_add) + if (!ctl_headers[TEST_H_EMPTY_ADD]) return -ENOMEM; /* Test that register on top of an empty dir works */ - sysctl_test_headers.empty + ctl_headers[TEST_H_EMPTY] = register_sysctl("debug/test_sysctl/empty_add/empty", test_table_empty); - if (!sysctl_test_headers.empty) + if (!ctl_headers[TEST_H_EMPTY]) return -ENOMEM; return 0; @@ -279,21 +280,21 @@ static const struct ctl_table table_u8_valid[] = { static int test_sysctl_register_u8_extra(void) { /* should fail because it's over */ - sysctl_test_headers.test_u8 + ctl_headers[TEST_H_U8] = register_sysctl("debug/test_sysctl", table_u8_over); - if (sysctl_test_headers.test_u8) + if (ctl_headers[TEST_H_U8]) return -ENOMEM; /* should fail because it's under */ - sysctl_test_headers.test_u8 + ctl_headers[TEST_H_U8] = register_sysctl("debug/test_sysctl", table_u8_under); - if (sysctl_test_headers.test_u8) + if (ctl_headers[TEST_H_U8]) return -ENOMEM; /* should not fail because it's valid */ - sysctl_test_headers.test_u8 + ctl_headers[TEST_H_U8] = register_sysctl("debug/test_sysctl", table_u8_valid); - if (!sysctl_test_headers.test_u8) + if (!ctl_headers[TEST_H_U8]) return -ENOMEM; return 0; @@ -321,18 +322,10 @@ module_init(test_sysctl_init); static void __exit test_sysctl_exit(void) { kfree(test_data.bitmap_0001); - if (sysctl_test_headers.test_h_setup_node) - unregister_sysctl_table(sysctl_test_headers.test_h_setup_node); - if (sysctl_test_headers.test_h_mnt) - unregister_sysctl_table(sysctl_test_headers.test_h_mnt); - if (sysctl_test_headers.test_h_mnterror) - unregister_sysctl_table(sysctl_test_headers.test_h_mnterror); - if (sysctl_test_headers.empty) - unregister_sysctl_table(sysctl_test_headers.empty); - if (sysctl_test_headers.empty_add) - unregister_sysctl_table(sysctl_test_headers.empty_add); - if (sysctl_test_headers.test_u8) - unregister_sysctl_table(sysctl_test_headers.test_u8); + for (int i = 0; i < TEST_H_SIZE; i++) { + if (ctl_headers[i]) + unregister_sysctl_table(ctl_headers[i]); + } } module_exit(test_sysctl_exit);