Re: [RFC PATCH v4 3/4] livepatch: Implement replace set for scoped atomic replace

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Dynamically allocated `funcs` arrays are leaked on KLP_OBSOLETES parsing failure.
--

--- Patch [3]: [RFC PATCH v4 3/4] livepatch: Implement replace set for scoped atomic replace ---
commit d186609c1a3dbe210857d0040f657fa918504faf
Author: Yafang Shao <[email protected]>
Subject: livepatch: Implement replace set for scoped atomic replace

This commit introduces a more flexible atomic replace model for livepatches
using provides and obsoletes fields. A new patch will replace any existing
patch with a matching provides ID or any ID listed in its obsoletes array.
The klp-build script and init code are updated to parse and validate these
new fields.

> diff --git a/scripts/livepatch/init.c b/scripts/livepatch/init.c
> --- a/scripts/livepatch/init.c
> +++ b/scripts/livepatch/init.c
[ ... ]
> @@ -72,10 +73,73 @@ static int __init livepatch_mod_init(void)
[ ... ]
> +#ifdef KLP_OBSOLETES
> +	/*
> +	 * Parse KLP_OBSOLETES string (format: "0,1,2" or "0" or empty)
> +	 * and convert to unsigned int array for patch->obsoletes
> +	 *
> +	 * Note: KLP_OBSOLETES is always defined and includes at least the
> +	 * provides ID, ensuring the patch replaces old patches with same 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;
> +		}

[Severity: Medium]
Does this code leak the funcs arrays on error?

Earlier in livepatch_mod_init(), the funcs arrays are dynamically allocated 
for each object in a loop:

	funcs = kzalloc(sizeof(struct klp_func) * (nr_funcs + 1), GFP_KERNEL);
	...
	obj->funcs = funcs;

If KLP_OBSOLETES parsing fails during string duplication, array allocation, 
or integer conversion, the error paths jump to err_free_objs. It appears 
that the cleanup label only frees the objs array itself but leaves the 
individual obj->funcs arrays allocated:

err_free_objs:
	kfree(objs);
err_free_patch:
	kfree(patch);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3