On Fri 2026-06-05 11:05:32, Marcos Paulo de Souza wrote: > On Mon, 2026-05-25 at 00:24 +0000, [email protected] 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 <[email protected]> > > > > 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
