| Message ID | 20250112215013.2386009-1-jsperbeck@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] sysctl: expose sysctl_check_table for unit testing and use it | expand |
Hi John, kernel test robot noticed the following build errors: [auto build test ERROR on kees/for-next/kspp] [also build test ERROR on linus/master v6.13-rc7 next-20250110] [cannot apply to kees/for-next/pstore sysctl/sysctl-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/John-Sperbeck/sysctl-expose-sysctl_check_table-for-unit-testing-and-use-it/20250113-055310 base: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/kspp patch link: https://lore.kernel.org/r/20250112215013.2386009-1-jsperbeck%40google.com patch subject: [PATCH v2] sysctl: expose sysctl_check_table for unit testing and use it config: csky-randconfig-002-20250113 (https://download.01.org/0day-ci/archive/20250113/202501131354.JbiHtAEH-lkp@intel.com/config) compiler: csky-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250113/202501131354.JbiHtAEH-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202501131354.JbiHtAEH-lkp@intel.com/ All errors (new ones prefixed by >>): In file included from kernel/sysctl-test.c:6: kernel/sysctl-test.c: In function 'sysctl_test_register_sysctl_sz_invalid_extra_value': >> kernel/sysctl-test.c:414:25: error: implicit declaration of function 'sysctl_check_table_test_helper' [-Wimplicit-function-declaration] 414 | sysctl_check_table_test_helper("foo", table_foo)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/kunit/test.h:774:22: note: in definition of macro 'KUNIT_BASE_BINARY_ASSERTION' 774 | const typeof(right) __right = (right); \ | ^~~~~ include/kunit/test.h:969:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION' 969 | KUNIT_BINARY_INT_ASSERTION(test, \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~ include/kunit/test.h:966:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG' 966 | KUNIT_EXPECT_EQ_MSG(test, left, right, NULL) | ^~~~~~~~~~~~~~~~~~~ kernel/sysctl-test.c:413:9: note: in expansion of macro 'KUNIT_EXPECT_EQ' 413 | KUNIT_EXPECT_EQ(test, -EINVAL, | ^~~~~~~~~~~~~~~ vim +/sysctl_check_table_test_helper +414 kernel/sysctl-test.c 369 370 /* 371 * Test that registering an invalid extra value is not allowed. 372 */ 373 static void sysctl_test_register_sysctl_sz_invalid_extra_value( 374 struct kunit *test) 375 { 376 unsigned char data = 0; 377 struct ctl_table table_foo[] = { 378 { 379 .procname = "foo", 380 .data = &data, 381 .maxlen = sizeof(u8), 382 .mode = 0644, 383 .proc_handler = proc_dou8vec_minmax, 384 .extra1 = SYSCTL_FOUR, 385 .extra2 = SYSCTL_ONE_THOUSAND, 386 }, 387 }; 388 389 struct ctl_table table_bar[] = { 390 { 391 .procname = "bar", 392 .data = &data, 393 .maxlen = sizeof(u8), 394 .mode = 0644, 395 .proc_handler = proc_dou8vec_minmax, 396 .extra1 = SYSCTL_NEG_ONE, 397 .extra2 = SYSCTL_ONE_HUNDRED, 398 }, 399 }; 400 401 struct ctl_table table_qux[] = { 402 { 403 .procname = "qux", 404 .data = &data, 405 .maxlen = sizeof(u8), 406 .mode = 0644, 407 .proc_handler = proc_dou8vec_minmax, 408 .extra1 = SYSCTL_ZERO, 409 .extra2 = SYSCTL_TWO_HUNDRED, 410 }, 411 }; 412 413 KUNIT_EXPECT_EQ(test, -EINVAL, > 414 sysctl_check_table_test_helper("foo", table_foo)); 415 KUNIT_EXPECT_EQ(test, -EINVAL, 416 sysctl_check_table_test_helper("foo", table_bar)); 417 KUNIT_EXPECT_EQ(test, 0, 418 sysctl_check_table_test_helper("foo", table_qux)); 419 } 420
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 27a283d85a6e..1de946176d74 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -1137,11 +1137,12 @@ static int sysctl_check_table_array(const char *path, const struct ctl_table *ta return err; } -static int sysctl_check_table(const char *path, struct ctl_table_header *header) +static int sysctl_check_table(const char *path, const struct ctl_table *table, + size_t table_size) { - const struct ctl_table *entry; + const struct ctl_table *entry = table; int err = 0; - list_for_each_table_entry(entry, header) { + for (size_t i = 0 ; i < table_size; ++i, entry++) { if (!entry->procname) err |= sysctl_err(path, entry, "procname is null"); if ((entry->proc_handler == proc_dostring) || @@ -1173,6 +1174,16 @@ static int sysctl_check_table(const char *path, struct ctl_table_header *header) return err; } +#ifdef CONFIG_KUNIT +int sysctl_check_table_test_helper_sz(const char *path, + const struct ctl_table *table, + size_t table_size) +{ + return sysctl_check_table(path, table, table_size); +} +EXPORT_SYMBOL(sysctl_check_table_test_helper_sz); +#endif /* CONFIG_KUNIT */ + static struct ctl_table_header *new_links(struct ctl_dir *dir, struct ctl_table_header *head) { struct ctl_table *link_table, *link; @@ -1372,6 +1383,9 @@ struct ctl_table_header *__register_sysctl_table( struct ctl_dir *dir; struct ctl_node *node; + if (sysctl_check_table(path, table, table_size)) + return NULL; + header = kzalloc(sizeof(struct ctl_table_header) + sizeof(struct ctl_node)*table_size, GFP_KERNEL_ACCOUNT); if (!header) @@ -1379,8 +1393,6 @@ struct ctl_table_header *__register_sysctl_table( node = (struct ctl_node *)(header + 1); init_header(header, root, set, node, table, table_size); - if (sysctl_check_table(path, header)) - goto fail; spin_lock(&sysctl_lock); dir = &set->dir; diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index 40a6ac6c9713..0f1d3a626f4f 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -247,6 +247,14 @@ extern int unaligned_enabled; extern int unaligned_dump_stack; extern int no_unaligned_warning; +#ifdef CONFIG_KUNIT +int sysctl_check_table_test_helper_sz(const char *path, + const struct ctl_table *table, + size_t table_size); +#define sysctl_check_table_test_helper(path, table) \ + sysctl_check_table_test_helper_sz(path, table, ARRAY_SIZE(table)) +#endif /* CONFIG_KUNIT */ + #else /* CONFIG_SYSCTL */ static inline void register_sysctl_init(const char *path, const struct ctl_table *table) diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c index 3ac98bb7fb82..247dd8536fc7 100644 --- a/kernel/sysctl-test.c +++ b/kernel/sysctl-test.c @@ -410,9 +410,12 @@ static void sysctl_test_register_sysctl_sz_invalid_extra_value( }, }; - 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)); + KUNIT_EXPECT_EQ(test, -EINVAL, + sysctl_check_table_test_helper("foo", table_foo)); + KUNIT_EXPECT_EQ(test, -EINVAL, + sysctl_check_table_test_helper("foo", table_bar)); + KUNIT_EXPECT_EQ(test, 0, + sysctl_check_table_test_helper("foo", table_qux)); } static struct kunit_case sysctl_test_cases[] = {
In commit b5ffbd139688 ("sysctl: move the extra1/2 boundary check of u8 to sysctl_check_table_array"), a kunit test was added that registers a sysctl table. If the test is run as a module, then a lingering reference to the module is left behind, and a 'sysctl -a' leads to a panic. This can be reproduced with these kernel config settings: 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 Instead of fully registering a sysctl table, expose the underlying checking function and use it in the unit test. Fixes: b5ffbd139688 ("sysctl: move the extra1/2 boundary check of u8 to sysctl_check_table_array") Signed-off-by: John Sperbeck <jsperbeck@google.com> --- fs/proc/proc_sysctl.c | 22 +++++++++++++++++----- include/linux/sysctl.h | 8 ++++++++ kernel/sysctl-test.c | 9 ++++++--- 3 files changed, 31 insertions(+), 8 deletions(-)