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

[Josh Poimboeuf <jpoimboe@kernel.org>]

On Tue, Aug 25, 2026 at 07:46:36PM +0800, Yafang Shao wrote:
> The current bool replace flag is too coarse: it is either all or
> nothing. A livepatch with .replace=true replaces ALL existing
> livepatches, which is safe but inflexible. There is no way to have
> multiple independent livepatch sets coexist on the same system.
> 
> Replace it with a more flexible model using two new fields in
> struct klp_patch:
> 
> - provides: an unsigned int id identifying the patch replace set.
>   By default (provides=0), any livepatch replaces any other livepatch.
> 
> - obsoletes: an optional array of unsigned int ids specifying
>   additional provides ids to be replaced. This allows a new patch
>   to explicitly obsolete patches from different replace sets.
> 
> A new livepatch atomically replaces any existing livepatch whose
> provides id matches either:
>   1. The new patch provides id (same replace set), or
>   2. Any id in the new patch obsoletes list
> 
> 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.
> 
> Two helper functions are introduced:
> - klp_patch_replaceable(): checks if an new patch will replace the old
>   patch

Should be klp_patch_replaces()?

> - klp_has_function_conflict(): rejects loading a livepatch that would
>   modify a function already patched by a livepatch with a different
>   provides id.

I'm not sure the commit log really needs to list some functions it adds,
that's already evident in the patch.

> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index ba9e3988c07c..854ed6230b96 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -123,7 +123,8 @@ struct klp_state {
>   * @mod:	reference to the live patch module
>   * @objs:	object entries for kernel objects to be patched
>   * @states:	system states that can get modified
> - * @replace:	replace all actively used patches
> + * @provides:	only one active livepatch per id
> + * @obsoletes:	replace given livepatch id(s)
>   * @list:	list node for global list of actively used patches
>   * @kobj:	kobject for sysfs resources
>   * @obj_list:	dynamic list of the object entries
> @@ -137,7 +138,9 @@ struct klp_patch {
>  	struct module *mod;
>  	struct klp_object *objs;
>  	struct klp_state *states;
> -	bool replace;
> +	unsigned int provides;
> +	unsigned int *obsoletes;
> +	unsigned int nr_obsoletes;

nr_obsoletes is missing from the kernel-doc above.

> @@ -72,15 +71,76 @@ static int __init livepatch_mod_init(void)
>  
>  	/* TODO patch->states */
>  
> -#ifdef KLP_NO_REPLACE
> -	patch->replace = false;
> +#ifdef KLP_PROVIDES
> +	patch->provides = KLP_PROVIDES;
>  #else
> -	patch->replace = true;
> +	patch->provides = 0;
> +#endif

'patch' is already zeroed, so the #else isn't needed.

> +#ifdef KLP_OBSOLETES
> +	/*
> +	 * Parse KLP_OBSOLETES string (format: "1,2,3") and convert to
> +	 * unsigned int array for patch->obsoletes.
> +	 *
> +	 * Note: The provides ID is not included here; the kernel will
> +	 * replace livepatch with the same provides ID.
> +	 */
> +	{
> +		unsigned int *obs_array;
> +		unsigned int count = 1;
> +		char *obsoletes_str;
> +		char *token, *str;
> +		int i = 0;
> +
> +		for (str = (char *)KLP_OBSOLETES; *str; str++) {
> +			if (*str == ',')
> +				count++;
> +		}
> +
> +		obsoletes_str = kstrdup(KLP_OBSOLETES, GFP_KERNEL);
> +		if (!obsoletes_str) {
> +			ret = -ENOMEM;
> +			goto err_free_objs;
> +		}
> +
> +		obs_array = kmalloc_array(count, sizeof(unsigned int), GFP_KERNEL);
> +		if (!obs_array) {
> +			kfree(obsoletes_str);
> +			ret = -ENOMEM;
> +			goto err_free_objs;
> +		}
> +
> +		str = obsoletes_str;
> +		while ((token = strsep(&str, ",")) != NULL) {
> +			unsigned int val;
> +
> +			ret = kstrtouint(token, 10, &val);
> +			if (ret) {
> +				kfree(obsoletes_str);
> +				kfree(obs_array);
> +				goto err_free_objs;
> +			}
> +			obs_array[i++] = val;
> +		}
> +
> +		patch->obsoletes = obs_array;
> +		patch->nr_obsoletes = i;
> +
> +		if (i > 0)
> +			pr_info("obsoletes patch ids: %s\n", KLP_OBSOLETES);
> +
> +		kfree(obsoletes_str);
> +	}

Can we do something simpler like this?

#ifdef KLP_OBSOLETES
static unsigned int klp_obsoletes[] = { KLP_OBSOLETES };
#endif

And in livepatch_mod_init():
	
#ifdef KLP_PROVIDES
	patch->provides = KLP_PROVIDES;
#endif
#ifdef KLP_OBSOLETES
	patch->obsoletes = klp_obsoletes;
	patch->nr_obsoletes = ARRAY_SIZE(klp_obsoletes);
#endif


> +++ b/scripts/livepatch/klp-build
> @@ -21,7 +21,8 @@ shopt -s lastpipe
>  
>  unset DEBUG_CLONE DIFF_CHECKSUM SKIP_CLEANUP VERBOSE XTRACE
>  
> -REPLACE=1
> +PROVIDES=0
> +OBSOLETES=""
>  SHORT_CIRCUIT=0
>  JOBS="$(getconf _NPROCESSORS_ONLN)"
>  shopt -o xtrace | grep -q 'on' && XTRACE=1
> @@ -132,7 +133,8 @@ Options:
>     -f, --show-first-changed	Show address of first changed instruction
>     -j, --jobs=<jobs>		Build jobs to run simultaneously [default: $JOBS]
>     -o, --output=<file.ko>	Output file [default: livepatch-<patch-name>.ko]
> -       --no-replace		Disable livepatch atomic replace
> +   -p, --provides=<id>		Set the provides id for this livepatch
> +   -r, --obsoletes=<ids>	Set the obsoletes ids array (e.g., "0,1,2")
>     -v, --verbose		Pass V=1 to kernel/module builds
>  
>  Advanced Options:
> @@ -147,7 +149,6 @@ Advanced Options:
>  
>  EOF
>  }
> -
>  usage() {

Unnecessary whitespace change

> @@ -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.
> +	if [[ -n "$OBSOLETES" ]]; then
> +		local obsoletes_clean="${OBSOLETES//[ ]/}"
> +		local IFS=','
> +		local -a obs_array=()
> +		local obs_id
> +		local already_exists
> +
> +		for obs_id in $obsoletes_clean; do
> +			if [[ -n "$obs_id" ]]; then
> +				already_exists=0
> +				for existing in "${obs_array[@]}"; do
> +					if [[ "$existing" -eq "$obs_id" ]]; then
> +						already_exists=1
> +						break
> +					fi
> +				done
> +
> +				if [[ "$already_exists" -eq 0 ]]; then
> +					obs_array+=("$obs_id")
> +				fi
> +			fi
> +		done
> +
> +		local IFS=','
> +		OBSOLETES="${obs_array[*]}"
> +		unset IFS
> +	fi
> +

What's the point of this?  Do we expect duplicates, and if so, wouldn't
it be the kernel's job to handle that?

-- 
Josh