Re: [PATCH v3 1/7] livepatch: Fix NULL pointer dereference in klp_find_func()

[Petr Mladek <pmladek@suse.com>]

On Sun 2026-06-07 21:16:53, Yafang Shao wrote:
> If a newly loaded livepatch provides a function entry with a NULL old_name,
> func->old_name will be NULL when evaluated in strcmp():
> 
> klp_init_patch()
>     klp_add_nops()
>         klp_find_func()
>             strcmp(old_func->old_name, func->old_name)
> 
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -92,7 +92,7 @@ static struct klp_func *klp_find_func(struct klp_object *obj,
>  		 * Besides identical old_sympos, also consider old_sympos
>  		 * of 0 and 1 are identical.
>  		 */
> -		if ((strcmp(old_func->old_name, func->old_name) == 0) &&
> +		if ((func->old_name && strcmp(old_func->old_name, func->old_name) == 0) &&

I do not have a good feeling about this solution because it quietly
ignores a problem. As a result, klp_add_object_nops() would
call klp_alloc_func_nop() even though it does not make much sense.

A livepatch where any func->oldname is not defined should get
rejected. It will actually happen but _later_ in:

  + klp_init_patch()
    + klp_init_object()
      + klp_init_func()

I see three better possibilities.

1. We could move/add the sanity checks into klp_init_patch_early()
   and return broken livepatches earlier.

2. We could move/add the sanity check into a new klp_check_patch()
   which will be called even before klp_init_patch_early().

3. We could allow klp_find_func() to return ERR_PTR(-EINVAL).
   klp_add_object_nops() should then return the error as well.

My preference:

I would do the 3rd variant because it is much easier than
adding/moving all consistency checks between klp_init_*() and
klp_init_*_early() or klp_check_*() functions.


>  		    ((old_func->old_sympos == func->old_sympos) ||
>  		     (old_func->old_sympos == 0 && func->old_sympos == 1) ||
>  		     (old_func->old_sympos == 1 && func->old_sympos == 0))) {

Best Regards,
Petr