On Mon, Jun 22, 2026 at 10:21 PM Miroslav Benes [off-list ref] wrote:

On 2026-06-09 15:27:18+02:00, Petr Mladek wrote:

quoted

On Sun 2026-06-07 21:16:53, Yafang Shao wrote:

quoted

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.

If you change it to do so.

quoted

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.

Interesting issue. If I remember correctly, klp_init_*() path is meant
for the initial checking originally (among others). That is why
klp_init_func() has it right at the beginning. It got a lot complicated
later. We introduced atomic replace feature which added klp_add_nops()
and is called earlier. Then there was the kobject issue which introduced
early init path. Both forgot about the check and that is what we have
now. I would prefer it to fix cleanly. klp_init_patch_early() is not
enough because klp_is_patch_compatible() is called even earlier. So your
option 2 sounds best to me. On the other hand, it might be difficult to
disentangle it all.

Anyway, could you send the fix separately since it is an existing issue,
please?

Does the following change look good to you ?

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 28d15ba58a26..317a3c866c76 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c

@@ -799,9 +799,6 @@ void klp_free_replaced_patches_async(struct

klp_patch *new_patch)

 static int klp_init_func(struct klp_object *obj, struct klp_func *func)
 {
-       if (!func->old_name)
-               return -EINVAL;
-
        /*
         * NOPs get the address later. The patched module must be loaded,
         * see klp_init_object_loaded().

@@ -1107,8 +1104,9 @@ static int __klp_enable_patch(struct klp_patch *patch)
  */
 int klp_enable_patch(struct klp_patch *patch)
 {
-       int ret;
        struct klp_object *obj;
+       struct klp_func *func;
+       int ret;

        if (!patch || !patch->mod || !patch->objs)
                return -EINVAL;

@@ -1116,9 +1114,12 @@ int klp_enable_patch(struct klp_patch *patch)
        klp_for_each_object_static(patch, obj) {
                if (!obj->funcs)
                        return -EINVAL;
+               klp_for_each_func_static(obj, func) {
+                       if (!func->old_name)
+                               return -EINVAL;
+               }
        }

-
        if (!is_livepatch_module(patch->mod)) {
                pr_err("module %s is not marked as a livepatch module\n",
                       patch->mod->name);

-- 
Regards
Yafang