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