fs/proc/proc_sysctl.c | 22 +++++++++++++++++----- include/linux/sysctl.h | 8 ++++++++ kernel/sysctl-test.c | 9 ++++++--- 3 files changed, 31 insertions(+), 8 deletions(-)
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(-)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 27a283d85a6e..2d3272826cc2 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;
}
+#if IS_ENABLED(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..09caac302333 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;
+#if IS_ENABLED(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[] = {
--
2.47.1.613.gc27f4b7a9f-googHi 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 sysctl/sysctl-next v6.13-rc7]
[cannot apply to kees/for-next/pstore]
[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-150214
base: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/kspp
patch link: https://lore.kernel.org/r/20250113070001.143690-1-jsperbeck%40google.com
patch subject: [PATCH v3] sysctl: expose sysctl_check_table for unit testing and use it
config: um-randconfig-002-20250118 (https://download.01.org/0day-ci/archive/20250118/202501182003.Gfi63jzH-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250118/202501182003.Gfi63jzH-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/202501182003.Gfi63jzH-lkp@intel.com/
All errors (new ones prefixed by >>):
>> kernel/sysctl-test.c:414:4: error: call to undeclared function 'sysctl_check_table_test_helper'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
sysctl_check_table_test_helper("foo", table_foo));
^
kernel/sysctl-test.c:416:4: error: call to undeclared function 'sysctl_check_table_test_helper'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
sysctl_check_table_test_helper("foo", table_bar));
^
kernel/sysctl-test.c:418:4: error: call to undeclared function 'sysctl_check_table_test_helper'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
sysctl_check_table_test_helper("foo", table_qux));
^
3 errors generated.
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
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>
---
The Change from v3 to v4 is to make sure sysctl_check_table_test_helper_sz()
is defined in the unusual case that the sysctl kunit test is enabled, but
CONFIG_SYSCTL is disabled.
fs/proc/proc_sysctl.c | 22 +++++++++++++++++-----
include/linux/sysctl.h | 17 +++++++++++++++++
kernel/sysctl-test.c | 9 ++++++---
3 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 27a283d85a6e..2d3272826cc2 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;
}
+#if IS_ENABLED(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..02acd3670bd2 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -288,4 +288,21 @@ static inline bool sysctl_is_alias(char *param)
int sysctl_max_threads(const struct ctl_table *table, int write, void *buffer,
size_t *lenp, loff_t *ppos);
+#if IS_ENABLED(CONFIG_KUNIT)
+#define sysctl_check_table_test_helper(path, table) \
+ sysctl_check_table_test_helper_sz(path, table, ARRAY_SIZE(table))
+#ifdef CONFIG_SYSCTL
+int sysctl_check_table_test_helper_sz(const char *path,
+ const struct ctl_table *table,
+ size_t table_size);
+#else /* CONFIG_SYSCTL */
+static inline int sysctl_check_table_test_helper_sz(const char *path,
+ const struct ctl_table *table,
+ size_t table_size)
+{
+ return 0;
+}
+#endif /* CONFIG_SYSCTL */
+#endif /* CONFIG_KUNIT */
+
#endif /* _LINUX_SYSCTL_H */
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[] = {
--
2.48.0.rc2.279.g1de40edade-googOn Tue, Jan 21, 2025 at 01:33:53PM -0800, John Sperbeck wrote:
> 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>
> ---
>
> The Change from v3 to v4 is to make sure sysctl_check_table_test_helper_sz()
> is defined in the unusual case that the sysctl kunit test is enabled, but
> CONFIG_SYSCTL is disabled.
>
> fs/proc/proc_sysctl.c | 22 +++++++++++++++++-----
> include/linux/sysctl.h | 17 +++++++++++++++++
> kernel/sysctl-test.c | 9 ++++++---
> 3 files changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 27a283d85a6e..2d3272826cc2 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++) {
This should be avoided as the traversal of the ctl_table should be
handled in one place (the list_for_each_table_entry macro)
> 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;
> }
>
> +#if IS_ENABLED(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..02acd3670bd2 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -288,4 +288,21 @@ static inline bool sysctl_is_alias(char *param)
> int sysctl_max_threads(const struct ctl_table *table, int write, void *buffer,
> size_t *lenp, loff_t *ppos);
>
> +#if IS_ENABLED(CONFIG_KUNIT)
> +#define sysctl_check_table_test_helper(path, table) \
> + sysctl_check_table_test_helper_sz(path, table, ARRAY_SIZE(table))
> +#ifdef CONFIG_SYSCTL
> +int sysctl_check_table_test_helper_sz(const char *path,
> + const struct ctl_table *table,
> + size_t table_size);
> +#else /* CONFIG_SYSCTL */
> +static inline int sysctl_check_table_test_helper_sz(const char *path,
> + const struct ctl_table *table,
> + size_t table_size)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SYSCTL */
> +#endif /* CONFIG_KUNIT */
> +
> #endif /* _LINUX_SYSCTL_H */
> 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));
This should all be in lib/tests_sysctl.c. We should remove all this
function from the kunit and add an equivalent one to lib/tests_sysctl.c
Best
> }
>
> static struct kunit_case sysctl_test_cases[] = {
> --
> 2.48.0.rc2.279.g1de40edade-goog
>
--
Joel Granados
On Sun, Jan 12, 2025 at 11:00:01PM -0800, John Sperbeck wrote:
> 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>
You have received a error from the bot. please address it to move
forward.
Best
> ---
> fs/proc/proc_sysctl.c | 22 +++++++++++++++++-----
> include/linux/sysctl.h | 8 ++++++++
> kernel/sysctl-test.c | 9 ++++++---
> 3 files changed, 31 insertions(+), 8 deletions(-)
>
...
>
> static struct kunit_case sysctl_test_cases[] = {
> --
> 2.47.1.613.gc27f4b7a9f-goog
>
--
Joel Granados
On Sun, Jan 12, 2025 at 11:00:01PM -0800, John Sperbeck wrote:
> 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.
thx for taking the time to change it. I have added it to my backlog and
will get to it at some point. It is not pressing as this touches sysctl
testing, which should not be used in the wild.
I know why you created the V3, but please add a comment on the mail (not
the commit) of what changed in every version.
Best
--
Joel Granados
© 2016 - 2025 Red Hat, Inc.