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

[Petr Mladek <pmladek@suse.com>]

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