Re: [PATCH v12 8/9] selftests/livepatch: Add state test for provides/obsoletes

[Petr Mladek <pmladek@suse.com>]

On Fri 2026-10-16 13:37:07, Yafang Shao wrote:
> Livepatches with different provides ids must not share the same state
> id, unless the second livepatch replaces the first one via obsoletes.
> 
> Add provides/obsoletes module parameters to test_klp_state and
> test_klp_state2, and cover both cases: reusing a state id across
> different provides ids is rejected, while it is allowed when the new
> livepatch obsoletes the old one.
> 
> --- a/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
> @@ -151,6 +165,16 @@ static struct klp_patch patch = {
>  
>  static int test_klp_callbacks_demo_init(void)
>  {
> +#ifdef CONFIG_KLP_HAS_PROVIDES
> +	patch.provides = provides;
> +	if (nr_obsoletes > 0) {
> +		patch.obsoletes = obsoletes;
> +		patch.nr_obsoletes = nr_obsoletes;
> +	}
> +#else
> +	patch.replace = true;

This changes the behavior. The original patch did not replace
others.

I believe that it would break the last test in test-sysfs.sh.
It passes with this patchset because the problematic test
is skipped. But it would break the test when it is used
for older kernels.

I guess that it is a cut-&-paste mistake ;-)

> +#endif
> +	states[0].id = state;
>  	return klp_enable_patch(&patch);
>  }
>  
> diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_state2.c b/tools/testing/selftests/livepatch/test_modules/test_klp_state2.c
> index 5f07fe1be2b9..aeb911b87d37 100644
> --- a/tools/testing/selftests/livepatch/test_modules/test_klp_state2.c
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_state2.c
> @@ -180,6 +194,16 @@ static struct klp_patch patch = {
>  
>  static int test_klp_callbacks_demo_init(void)
>  {
> +#ifdef CONFIG_KLP_HAS_PROVIDES
> +	patch.provides = provides;
> +	if (nr_obsoletes > 0) {
> +		patch.obsoletes = obsoletes;
> +		patch.nr_obsoletes = nr_obsoletes;
> +	}
> +#else
> +	patch.replace = true;

Same here:

> +#endif
> +	states[0].id = state;
>  	return klp_enable_patch(&patch);
>  }

Otherwise, it looks good to me and passes the test.

With the "patch.replace = true;" lines removed:

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr