Re: [PATCH v7 for-next 3/8] livepatch: Implement replace set for scoped atomic replace

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Contradiction between documentation/commit message and actual code regarding the 'obsoletes' list.
--

--- Patch [3]: [PATCH v7 for-next 3/8] livepatch: Implement replace set for scoped atomic replace ---
commit 32e1bcc9ed17f1580951816746e0cfa7cadbca0b
Author: Yafang Shao <laoar.shao@gmail.com>

livepatch: Implement replace set for scoped atomic replace

This commit replaces the boolean replace flag with a more flexible model
using provides and obsoletes fields to allow multiple independent livepatch
sets to coexist on the same system.

> The klp-build script is updated with -p/--provides and -r/--obsoletes
> options. The obsoletes list automatically includes the provides id
> and deduplicates entries. Input validation rejects malformed values
> at build time.

[Severity: Medium]
Is this description accurate? The code and inline comments in the build
script and init.c explicitly state that the provides ID is not included in
the obsoletes list, and the kernel instead relies on explicitly checking the
provides ID directly.

> diff --git a/Documentation/livepatch/cumulative-patches.rst b/Documentation/livepatch/cumulative-patches.rst
> index 1931f318976a5..04352ae3f0d06 100644
> --- a/Documentation/livepatch/cumulative-patches.rst
> +++ b/Documentation/livepatch/cumulative-patches.rst

[ ... ]

> +Provides and Obsoletes
> +-----------------------
> +
> +The ``provides`` field in ``struct klp_patch`` is an unsigned integer that
> +identifies the livepatch's replace set. By default, it is 0.
> +
> +The ``obsoletes`` field is an optional array of unsigned integers that
> +specifies additional ``provides`` ids to be replaced when this patch is
> +loaded. By default, it includes the patch's own ``provides`` id, ensuring
> +that a new patch always replaces any existing patch with the same
> +``provides`` id.

[Severity: Medium]
This isn't a code bug, but does this documentation match the implementation?
The text here states that the obsoletes field automatically includes the
patch's own provides id by default. 

> diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> index b52a8489d9f67..64bf3fce7fbc1 100755
> --- a/scripts/livepatch/klp-build
> +++ b/scripts/livepatch/klp-build

[ ... ]

> @@ -235,6 +254,37 @@ process_args() {
>  		exit 1
>  	fi
>  
> +	# Remove duplicates from obsoletes (if specified)
> +	# Note: provides ID is not added here; the kernel will replace
> +	# livepatch with the same provides ID.

[Severity: Medium]
Here the build script explicitly notes that the provides ID is not added to
the obsoletes list. Could the commit message and the documentation in
cumulative-patches.rst be updated to match this behavior to avoid confusion?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825114641.80452-1-laoar.shao@gmail.com?part=3