summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJoel Granados <joel.granados@kernel.org>2025-03-13 22:35:25 +0100
committerJoel Granados <joel.granados@kernel.org>2025-04-14 14:13:41 +0200
commit138303ec6ccbe38611931eeb955a722c6f78ec25 (patch)
tree478c53bd05ccbd294b28f1d2c271aac7b71abfa5
parentbc4f328ff516f6ea53c3c0d385a84ea7ae423c20 (diff)
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: <TASK> 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 <kees@kernel.org> Signed-off-by: Joel Granados <joel.granados@kernel.org>
-rw-r--r--kernel/sysctl-test.c49
-rw-r--r--lib/test_sysctl.c66
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);