From: Kees Cook <kees@kernel.org>
To: Joel Granados <joel.granados@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Shuah Khan <shuah@kernel.org>,
John Sperbeck <jsperbeck@google.com>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kselftest@vger.kernel.org
Subject: Re: [PATCH 1/4] sysctl: move u8 register test to lib/test_sysctl.c
Date: Wed, 9 Apr 2025 10:26:56 -0700 [thread overview]
Message-ID: <202504091020.3A06E6C548@keescook> (raw)
In-Reply-To: <20250321-jag-test_extra_val-v1-1-a01b3b17dc66@kernel.org>
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
next prev parent reply other threads:[~2025-04-09 17:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-21 12:47 [PATCH 0/4] sysctl: Move the u8 range check test to lib/test_sysctl.c Joel Granados
2025-03-21 12:47 ` [PATCH 1/4] sysctl: move u8 register " Joel Granados
2025-04-09 17:26 ` Kees Cook [this message]
2025-04-11 12:37 ` Joel Granados
2025-03-21 12:47 ` [PATCH 2/4] sysctl: Add 0012 to test the u8 range check Joel Granados
2025-04-09 17:27 ` Kees Cook
2025-03-21 12:47 ` [PATCH 3/4] sysctl: call sysctl tests with a for loop Joel Granados
2025-04-09 17:28 ` Kees Cook
2025-03-21 12:47 ` [PATCH 4/4] sysctl: Close test ctl_headers " Joel Granados
2025-04-09 17:29 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202504091020.3A06E6C548@keescook \
--to=kees@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=joel.granados@kernel.org \
--cc=jsperbeck@google.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox