Thread (50 messages) 50 messages, 5 authors, 7d ago

Re: [PATCH v3 3/7] livepatch: Support scoped atomic replace using replace_set

From: sashiko-bot@kernel.org
Date: 2026-06-07 13:33:14

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 hunk ↗ jump to hunk
--- a/kernel/livepatch/state.c
+++ b/kernel/livepatch/state.c
[ ... ]
+		/*
+		 * 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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260607131659.29281-1-laoar.shao@gmail.com?part=3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help