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@xxxxxxxxxx>

Otherwise looks good to me.

Reviewed-by: Kees Cook <kees@xxxxxxxxxx>

(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