Re: [PATCH v2 2/4] livepatch: Support scoped atomic replace using replace_set

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A malformed livepatch module with a missing function `old_name` will cause a kernel panic during patch compatibility checking.
--

commit 48dd0b716d1f70f13c1e96b75bf54e1aba77be18
Author: Yafang Shao <laoar.shao@gmail.com>

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. There can only ever be one active
livepatch for a given replace_set number.

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

> diff --git a/kernel/livepatch/state.c b/kernel/livepatch/state.c
> index 2565d039ade0a..a1ac466373360 100644
> --- a/kernel/livepatch/state.c
> +++ b/kernel/livepatch/state.c

[ ... ]

> @@ -85,34 +85,65 @@ EXPORT_SYMBOL_GPL(klp_get_prev_state);

[ ... ]

> +		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]
Could a malformed livepatch module cause a kernel panic here during patch
compatibility checking?

Since klp_is_patch_compatible() is called in klp_enable_patch() before
klp_init_patch() validates the incoming patch structures, a malformed patch
might have func->old_name set to NULL but func->new_func populated.

In that scenario, klp_for_each_func_static() would still process the function
and pass it to klp_find_func(), where it is used as old_func:

kernel/livepatch/core.c:klp_find_func() {
    ...
    klp_for_each_func(obj, func) {
        if ((strcmp(old_func->old_name, func->old_name) == 0) &&
    ...
}

Would this result in a NULL pointer dereference when klp_find_func() attempts
to call strcmp() on the unvalidated function's old_name? While livepatch
modules are loaded by root, should they fail validation gracefully instead of
crashing the kernel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529034542.68766-1-laoar.shao@gmail.com?part=2