Re: [PATCH 3/4] selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels

[Petr Mladek <pmladek@suse.com>]

On Fri 2026-06-05 11:05:32, Marcos Paulo de Souza wrote:
> On Mon, 2026-05-25 at 00:24 +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue(s) to consider:
> > - [Medium] The `test_klp_mod_target_ops` structure omits the
> > logically required `.set` callback, exposing the kernel to a NULL
> > pointer dereference during module load.
> > --
> > 
> > commit 532c1b8c7438f534fd4e3440716ede6cf83efe1c
> > Author: Marcos Paulo de Souza <mpdesouza@suse.com>
> > 
> > selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels
> > 
> > Use the stable module_param_cb API instead of proc_fs for exposing
> > module
> > state. This approach is compatible with kernel 4.12 and later.
> > 
> > > diff --git
> > > a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_targe
> > > t.c
> > > b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_targe
> > > t.c
> > > index 9643984d24022..1382a266825fb 100644
> > > ---
> > > a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_targe
> > > t.c
> > > +++
> > > b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_targe
> > > t.c
> > > @@ -5,31 +5,29 @@
> > [ ... ]
> > > +static const struct kernel_param_ops test_klp_mod_target_ops = {
> > > +	.get = test_klp_mod_target_get,
> > > +};
> > > +
> > > +module_param_cb(klp_mod_arg, &test_klp_mod_target_ops, NULL,
> > > 0444);
> > 
> > [Severity: Medium]
> > Does this code need a .set callback in the kernel_param_ops
> > structure?
> > 
> > When a module parameter is passed during module load, the kernel
> > parses the
> > arguments using parse_args(), which invokes parse_one() in
> > kernel/params.c.
> > Because parse_one() unconditionally executes the .set callback
> > without
> > verifying if it is actually defined, omitting it here leaves the
> > callback
> > implicitly initialized to NULL.
> > 
> > Could this lead to a NULL pointer dereference and a kernel panic if
> > someone
> > attempts to set this parameter during module load?
> 
> Indeed! I'll send a v2 addressing the missing argument. Thanks Sachiko
> for spotting it :)

It should be perfectly fine to use NULL for the .set() callback.
It can't be called because of the given permissions 0444.

We should teach Sashiko about this ;-)

Best Regards,
Petr