Re: [PATCH v2] livepatch: Fix NULL pointer dereference in klp_find_func()

[Petr Mladek <pmladek@suse.com>]

On Sun 2026-06-28 19:46:35, Yafang Shao wrote:
> A NULL old_name in a newly loaded livepatch's function entry causes a
> NULL pointer dereference in strcmp():
> 
>   klp_init_patch()
>     klp_add_nops()
>       klp_find_func()
>         strcmp(old_func->old_name, func->old_name)
> 
> Add klp_check_patch() at the beginning of klp_enable_patch() to reject
> patches with NULL old_name before they reach this code path.
> 
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/live-patching/20260529040130.95A9C1F00893@smtp.kernel.org/
> Suggested-by: Petr Mladek <pmladek@suse.com>
> Suggested-by: Miroslav Benes <mbenes@suse.cz>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Acked-by: Miroslav Benes <mbenes@suse.cz>

Looks good to me:

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

I think whether to rush this into 7.2-rcX or wait for 7.3.
On one hand, it is a possible security fix. On the other hand,
broken module could never get loaded and pass even a minimal QA.

I tend to push it for 7.2-rcX just to avoid questions from people
who ignore the real life vulnerabilty aspect.

Also see a nit below.

> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -1092,6 +1089,25 @@ static int __klp_enable_patch(struct klp_patch *patch)
>  	return ret;
>  }
>  
> +static int klp_check_patch(struct klp_patch *patch)
> +{
> +	struct klp_object *obj;
> +	struct klp_func *func;
> +
> +	if (!patch || !patch->mod || !patch->objs)
> +		return -EINVAL;
> +
> +	klp_for_each_object_static(patch, obj) {
> +		if (!obj->funcs)
> +			return -EINVAL;

Nit: I would add an empty line here to make it better readable.
     Of course, this is a personal preference. But it seems that
     most contributors to this code prefer visual separators
     between misc code sections as well.

     No need for v3. I could add the empty line when pushing
     the change.

> +		klp_for_each_func_static(obj, func) {
> +			if (!func->old_name)
> +				return -EINVAL;
> +		}
> +	}
> +	return 0;
> +}
> +
>  /**
>   * klp_enable_patch() - enable the livepatch
>   * @patch:	patch to be enabled

Best Regards,
Petr