[PATCH 0/4] sysctl: Move the u8 range check test to lib/test_sysctl.c

[PATCH 0/4] sysctl: Move the u8 range check test to lib/test_sysctl.c

[Joel Granados]

Originally introduced to sysctl-test.c by commit b5ffbd139688 ("sysctl:
move the extra1/2 boundary check of u8 to sysctl_check_table_array"), it
has been shown to lead to a panic under certain conditions related to a
dangling registration.

This series moves the u8 test to lib/test_sysctl.c where the
registration calls are kept and correctly removed on module exit. An
additional 0012 test is added to selftests/sysctl/sysctl.sh in order to
visualize the registration calls done in test_sysctl.c.

Very much related to adding tests to sysctl, the last two patches of
this series reduce the places that need to be changed when tests are
added by managing the initialization and closing of sysctl tables with a
for loop.

Comments are greatly appreciated

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
Joel Granados (4):
      sysctl: move u8 register test to lib/test_sysctl.c
      sysctl: Add 0012 to test the u8 range check
      sysctl: call sysctl tests with a for loop
      sysctl: Close test ctl_headers with a for loop

 kernel/sysctl-test.c                     |  49 ------------
 lib/test_sysctl.c                        | 133 +++++++++++++++++++++----------
 tools/testing/selftests/sysctl/sysctl.sh |  30 +++++++
 3 files changed, 122 insertions(+), 90 deletions(-)
---
base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
change-id: 20250321-jag-test_extra_val-40954050a1f6

Best regards,
-- 
Joel Granados <joel.granados@kernel.org>

[PATCH 1/4] sysctl: move u8 register test to lib/test_sysctl.c

[Joel Granados]

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")'

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
 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 eb2842bd055771ee8a72040f96260bdb0e754ad5..92f94ea28957f48893b0d0d947d73001428fecc7 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 4249e0cc8aaff79d7da03aff81b3f9990e9eaac1..54a22e4b134677e022af05df3c75268e7a4a79e7 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);

-- 
2.47.2

Re: [PATCH 1/4] sysctl: move u8 register test to lib/test_sysctl.c

[Kees Cook]

On Fri, Mar 21, 2025 at 01:47:24PM +0100, Joel Granados wrote:
> 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

Typoe: drop leading '

> sysctl_check_table_array")'

And avoid wrapping this line for the field.

> 
> Signed-off-by: Joel Granados <joel.granados@kernel.org>

Otherwise looks good to me.

Reviewed-by: Kees Cook <kees@kernel.org>

(And I should  note that there is a push to move kunit tests into a
"/tests/" subdir, but that's separate from this series.)

-- 
Kees Cook

Re: [PATCH 1/4] sysctl: move u8 register test to lib/test_sysctl.c

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]

On Wed, Apr 09, 2025 at 10:26:56AM -0700, Kees Cook wrote:
> On Fri, Mar 21, 2025 at 01:47:24PM +0100, Joel Granados wrote:
> > 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
> 
> Typoe: drop leading '
> 
> > sysctl_check_table_array")'
> 
> And avoid wrapping this line for the field.
> 
> > 
> > Signed-off-by: Joel Granados <joel.granados@kernel.org>
> 
> Otherwise looks good to me.

Thx for the feedback; 
Changed this and took in your trailers, but wont resend.

Best
-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 2/4] sysctl: Add 0012 to test the u8 range check

[Joel Granados]

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.

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
 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 84472b436c07faf43e555999951e554f8e1a60c5..22b53c263dd4525ee8d4655272d6842e5249a7dc 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()

-- 
2.47.2

Re: [PATCH 2/4] sysctl: Add 0012 to test the u8 range check

[Kees Cook]

On Fri, Mar 21, 2025 at 01:47:25PM +0100, Joel Granados wrote:
> 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.
> 
> Signed-off-by: Joel Granados <joel.granados@kernel.org>

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

[PATCH 3/4] sysctl: call sysctl tests with a for loop

[Joel Granados]

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.

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
 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 54a22e4b134677e022af05df3c75268e7a4a79e7..4b3d56de6269b93220ecbeb3d3d4e42944b0ca78 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);

-- 
2.47.2

Re: [PATCH 3/4] sysctl: call sysctl tests with a for loop

[Kees Cook]

On Fri, Mar 21, 2025 at 01:47:26PM +0100, Joel Granados wrote:
> 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.
> 
> Signed-off-by: Joel Granados <joel.granados@kernel.org>

Nice cleanup!

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

[PATCH 4/4] sysctl: Close test ctl_headers with a for loop

[Joel Granados]

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.

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
 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 4b3d56de6269b93220ecbeb3d3d4e42944b0ca78..c02aa9c868f2117606b24f114326bf1c396cd584 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);

-- 
2.47.2

Re: [PATCH 4/4] sysctl: Close test ctl_headers with a for loop

[Kees Cook]

On Fri, Mar 21, 2025 at 01:47:27PM +0100, Joel Granados wrote:
> 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.
> 
> Signed-off-by: Joel Granados <joel.granados@kernel.org>

Much cleaner too. :)

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook