linux-kernel - [PATCH] sysctl: unregister sysctl table after testing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241224171124.3676538-1-jsperbeck@google.com>
Date: Tue, 24 Dec 2024 09:11:24 -0800
From: John Sperbeck <jsperbeck@...gle.com>
To: Joel Granados <joel.granados@...nel.org>, Kees Cook <kees@...nel.org>
Cc: Wen Yang <wen.yang@...ux.dev>, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, John Sperbeck <jsperbeck@...gle.com>
Subject: [PATCH] sysctl: unregister sysctl table after testing

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

If we unregister the test sysctl table, then the failure is gone.

Fixes: b5ffbd139688 ("sysctl: move the extra1/2 boundary check of u8 to sysctl_check_table_array")
Signed-off-by: John Sperbeck <jsperbeck@...gle.com>
---
 kernel/sysctl-test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
index 3ac98bb7fb82..2184c1813b1d 100644
--- a/kernel/sysctl-test.c
+++ b/kernel/sysctl-test.c
@@ -373,6 +373,7 @@ static void sysctl_test_api_dointvec_write_single_greater_int_max(
 static void sysctl_test_register_sysctl_sz_invalid_extra_value(
 		struct kunit *test)
 {
+	struct ctl_table_header *hdr;
 	unsigned char data = 0;
 	struct ctl_table table_foo[] = {
 		{
@@ -412,7 +413,9 @@ 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));
+	hdr = register_sysctl("foo", table_qux);
+	KUNIT_EXPECT_NOT_NULL(test, hdr);
+	unregister_sysctl_table(hdr);
 }
 
 static struct kunit_case sysctl_test_cases[] = {
-- 
2.47.1.613.gc27f4b7a9f-goog