On Sun, Jun 7, 2026 at 9:33 PM [off-list ref] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly introduced check in `klp_is_patch_compatible` causes a kernel panic if a livepatch module contains a `klp_func` entry with `old_name == NULL`.
--

commit ed678e88294b650a730cda1a47351fce99677c1e
Author: Yafang Shao [off-list ref]

livepatch: Support scoped atomic replace using replace_set

Convert the replace attribute from a boolean to a u32 to function as a
"replace set." A newly loaded livepatch will now atomically replace any
existing patch belonging to the same set.

Link: https://github.com/pmladek/linux/tree/klp-state-transfer-v1-iter12 [0]

[ ... ]

quoted

--- a/kernel/livepatch/state.c
+++ b/kernel/livepatch/state.c

[ ... ]

quoted

+             /*
+              * Refuse loading a livepatch which would want to modify a
+              * function which is already livepatched with the livepatch
+              * with another "replace_set".
+              */
+             klp_for_each_object_static(patch, obj) {
+                     klp_for_each_object(old_patch, old_obj) {
+                             if (!!obj->name != !!old_obj->name)
+                                     continue;
+                             if (obj->name && strcmp(obj->name, old_obj->name))
+                                     continue;
+                             klp_for_each_func_static(obj, func) {
+                                     if (klp_find_func(old_obj, func))
+                                             return false;
+                             }
+                     }
+             }
      }

[Severity: High]
Can passing an unvalidated function to klp_find_func() cause a NULL pointer
dereference here?

If a new livepatch module is loaded with an improperly initialized klp_func
array (e.g., accidentally missing old_name but providing new_func),
klp_for_each_func_static() will still process it because structural
validation in klp_init_patch() hasn't occurred yet at this point in the
loading process.

When this unvalidated func is passed as the second argument to
klp_find_func(old_obj, func), the roles inside klp_find_func() are reversed
from its usual context. The old_func parameter represents the new patch's
unvalidated function, while the local func iterator represents the
already-installed patch.

Inside klp_find_func(), the check evaluates:

    if ((func->old_name && strcmp(old_func->old_name, func->old_name) == 0)

Since the installed patch's func->old_name is guaranteed to be non-NULL,
strcmp() will be called with old_func->old_name. If old_func->old_name is
NULL, does this turn what would normally be a graceful -EINVAL module
rejection into a system crash?

Oops, I missed that. It seems we should better check both func->name
and old_func->name in patch #1.

--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c

@@ -82,20 +82,22 @@ static bool klp_initialized(void)
        return !!klp_root_kobj;
 }

+/* Check if @func is present in @obj */
 static struct klp_func *klp_find_func(struct klp_object *obj,
-                                     struct klp_func *old_func)
+                                     struct klp_func *func)
 {
-       struct klp_func *func;
+       struct klp_func *obj_func;

-       klp_for_each_func(obj, func) {
+       klp_for_each_func(obj, obj_func) {
                /*
                 * Besides identical old_sympos, also consider old_sympos
                 * of 0 and 1 are identical.
                 */
-               if ((strcmp(old_func->old_name, func->old_name) == 0) &&
-                   ((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))) {
+               if ((obj_func->old_name && func->old_name &&
+                    strcmp(obj_func->old_name, func->old_name) == 0) &&
+                   ((obj_func->old_sympos == func->old_sympos) ||
+                    (obj_func->old_sympos == 0 && func->old_sympos == 1) ||
+                    (obj_func->old_sympos == 1 && func->old_sympos == 0))) {
                        return func;
                }
        }

-- 
Regards
Yafang