Re: [RFC PATCH v4 1/4] livepatch: Make klp_find_func() non static

On Tue, Aug 4, 2026 at 2:56 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] klp_find_func() incorrectly returns the search parameter instead of the matched object from the list.
> --
>
> --- Patch [1]: [RFC PATCH v4 1/4] livepatch: Make klp_find_func() non static ---
> commit 59916304ff12f7685cb556f7a7c1f4a1c5a25b00
> Author: Yafang Shao <[email protected]>
>
> livepatch: Make klp_find_func() non static
>
> This commit makes klp_find_func() non static so it can be used in other
> source files by an upcoming patch. It also renames the old_func parameter
> to func to better reflect its generic purpose.
>
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index 28d15ba58a26d..e07c41a0f3ac8 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> [ ... ]
> > -static struct klp_func *klp_find_func(struct klp_object *obj,
> > -                                   struct klp_func *old_func)
> > +/* Check if @func is present in @obj. */
> > +struct klp_func *klp_find_func(struct klp_object *obj, struct klp_func *func)
> >  {
> > -     struct klp_func *func;
> > +     struct klp_func *obj_func;
> >
> > -     klp_for_each_func(obj, func) {
> > +     klp_for_each_func(obj, obj_func) {
> >               /*
> >                * Besides identical old_sympos, also consider old_sympos
> >                * of 0 and 1 are identical.
> >                */
> > -             if ((strcmp(old_func->old_name, func->old_name) == 0) &&
> > -                 ((old_func->old_sympos == func->old_sympos) ||
> > -                  (old_func->old_sympos == 0 && func->old_sympos == 1) ||
> > -                  (old_func->old_sympos == 1 && func->old_sympos == 0))) {
> > +             if ((strcmp(obj_func->old_name, func->old_name) == 0) &&
> > +                 ((obj_func->old_sympos == func->old_sympos) ||
> > +                  (obj_func->old_sympos == 0 && func->old_sympos == 1) ||
> > +                  (obj_func->old_sympos == 1 && func->old_sympos == 0))) {
> >                       return func;
>
> [Severity: Medium]
> Will this return the correct object? It looks like this now returns the
> search parameter (func) instead of the newly introduced list iterator
> variable (obj_func).
>
> If a caller relies on the returned pointer to access the matched object
> from the list, they will receive the search parameter instead. This could
> lead to incorrect state manipulation or memory corruption for new users
> of this exported function.
>
> Should this return obj_func instead?

Good catch. It should return obj_func instead.

-- 
Regards
Yafang