Re: [PATCH v2 1/4] livepatch: Make klp_find_func() non-static

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] klp_find_func() is improperly exposed in the global public header, breaking encapsulation and introducing potential build/linkage errors.

Pre-existing issues:
- [High] klp_find_func() lacks input validation, exacerbating a pre-existing NULL pointer dereference vulnerability during livepatch loading.
--

commit 0b93025e6e1dd48057bb98e4d5e13741daab118e
Author: Yafang Shao <laoar.shao@gmail.com>

livepatch: Make klp_find_func() non-static
    
This commit exposes klp_find_func() by moving it out of the static scope
to make it available outside of core.c for reuse in upcoming patches.

> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index ba9e3988c07c3..70854f542c33e 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -215,6 +215,10 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
>  			     unsigned int symindex, unsigned int secindex,
>  			     const char *objname);
>  
> +struct klp_func *klp_find_func(struct klp_object *obj,
> +			       struct klp_func *old_func);
> +

[Severity: Low]
Does this declaration belong in kernel/livepatch/core.h instead? It lacks
an EXPORT_SYMBOL_GPL() and a stub for when !CONFIG_LIVEPATCH is set.

[ ... ]

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 28d15ba58a26d..e97df3e59057b 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -82,8 +82,8 @@ static bool klp_initialized(void)
>  	return !!klp_root_kobj;
>  }
>  
> -static struct klp_func *klp_find_func(struct klp_object *obj,
> -				      struct klp_func *old_func)
> +struct klp_func *klp_find_func(struct klp_object *obj,
> +			       struct klp_func *old_func)
>  {
>  	struct klp_func *func;

[Severity: High]
This is a pre-existing issue, but does this function dereference a NULL
pointer if old_name is NULL?

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)

Could a check be added for NULL before calling strcmp()?

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