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 <[email protected]> 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] [ ... ] > --- 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/[email protected]?part=3
