On Sun 2026-05-24 20:50:32, Marcos Paulo de Souza wrote:
> 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. The end
> result is the same: the module has a function that shows a string, which
> is later livepatched to show a different string. The only difference is
> that the file being checked is now a module parameter instead of a
> procfs entry.
>
> --- a/tools/testing/selftests/livepatch/test-livepatch.sh
> +++ b/tools/testing/selftests/livepatch/test-livepatch.sh
> @@ -198,26 +198,25 @@ livepatch: '$MOD_REPLACE': unpatching complete
> % rmmod $MOD_REPLACE"
>
>
> -# - load a target module that provides /proc/test_klp_mod_target with
> -# original output
> -# - load a livepatch that patches the target module's show function
> -# - verify the proc entry returns livepatched output
> +# - load a target module with module_param_cb get data with original output
> +# - load a livepatch that patches the target module's get function
> +# - verify the parameter get function returns the livepatched output
> # - disable and unload the livepatch
> -# - verify the proc entry returns original output again
> +# - verify the parameter get function returns the original output again
> # - unload the target module
>
> start_test "module function patching"
>
> load_mod $MOD_TARGET
>
> -if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
> +if [[ "$(cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/klp_mod_arg)" !=
> "$MOD_TARGET: original output" ]] ; then
I would personally call the parameter "test" or something simple.
A better descriptive name was needed in the top-level /proc directory
to prevent conflicts. But the module-specific parameter is
checked module-specific directory...
> echo -e "FAIL\n\n"
> die "livepatch kselftest(s) failed"
> fi
>
> --- a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c
> @@ -8,17 +8,16 @@
> #include <linux/livepatch.h>
> #include <linux/seq_file.h>
>
> -static int livepatch_mod_target_show(struct seq_file *m, void *v)
> +static int livepatch_mod_target_get(char *buffer, const struct kernel_param
> *kp)
> {
> - seq_printf(m, "%s: %s\n", THIS_MODULE->name,
> - "this has been live patched");
> - return 0;
> + return sprintf(buffer, "%s: %s\n", THIS_MODULE->name,
> + "this has been live patched");
Please, use sysfs_emit() instead of sprintf(). It is a specifically
designed API for sysfs output. It prevents a potential buffer overflow.
It is less important here. But we should show good examples...
> }
>
> static struct klp_func funcs[] = {
> {
> - .old_name = "test_klp_mod_target_show",
> - .new_func = livepatch_mod_target_show,
> + .old_name = "test_klp_mod_target_get",
> + .new_func = livepatch_mod_target_get,
> },
> {},
> };
Otherwise the change looks good to me.
Best Regards,
Petr
