Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The `replace_set` module parameter is defined as `int` with `0644` permissions, but the `replace_set` field in `struct klp_patch` is an `unsigned int` and cannot be dynamically modified post-initialization. --
commit 4cfbab500868bad0b10a33bdcbb852806612d546 Author: Yafang Shao <[email protected]> selftests/livepatch: Update tests for replace_set This commit updates the livepatch selftests to replace the replace flag with replace_set. It adjusts test cases to configure multiple livepatches that share either the same or different replace_set values, and removes the test case where an atomic replace disables multiple existing livepatches. [ ... ] > diff --git > a/tools/testing/selftests/livepatch/test_modules/test_klp_atomic_replace.c > b/tools/testing/selftests/livepatch/test_modules/test_klp_atomic_replace.c > index 5af7093ca00c4..5333503f193a3 100644 > --- a/tools/testing/selftests/livepatch/test_modules/test_klp_atomic_replace.c > +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_atomic_replace.c > @@ -7,9 +7,9 @@ > #include <linux/kernel.h> > #include <linux/livepatch.h> > > -static int replace; > -module_param(replace, int, 0644); > -MODULE_PARM_DESC(replace, "replace (default=0)"); > +static int replace_set; > +module_param(replace_set, int, 0644); [Severity: Low] The replace_set field in struct klp_patch is an unsigned int, and it cannot be dynamically modified after initialization. Should this module parameter be defined as uint with read-only permissions (0444) instead of int and 0644? Using int and 0644 allows negative values which could cause improper implicit casts, and runtime modifications have no actual effect on the livepatch. [ ... ] > diff --git > a/tools/testing/selftests/livepatch/test_modules/test_klp_livepatch.c > b/tools/testing/selftests/livepatch/test_modules/test_klp_livepatch.c > index aff08199de712..fedd2494d187f 100644 > --- a/tools/testing/selftests/livepatch/test_modules/test_klp_livepatch.c > +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_livepatch.c > @@ -15,6 +15,10 @@ static int livepatch_cmdline_proc_show(struct seq_file *m, > void *v) > return 0; > } > > +static int replace_set; > +module_param(replace_set, int, 0644); [Severity: Low] Does this parameter also need to be updated to uint and 0444? As noted above, the underlying struct klp_patch replace_set field is an unsigned int and is unaffected by modifications after initialization. [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
