Re: [PATCH v3 5/7] selftests/livepatch: Update tests for replace_set

[sashiko-bot@kernel.org]

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 <laoar.shao@gmail.com>

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/20260607131659.29281-1-laoar.shao@gmail.com?part=5