Re: [PATCH] livepatch: Improve the accuracy of symbol search

[Miroslav Benes <mbenes@suse.cz>]

Hi,

thank you for the patch...

> module_kallsyms_on_each_symbol, when the input parameter modname is not
>  empty, only searches for symbols within the current module.

Yes, correct.

> When
> patching a kernel object (ko), if the patched function calls
> functions from vmlinux or other ko modules, symbol lookup may fail.

dtto, expected behaviour.

> When patching a ko, the current approach first searches for symbols
> within the module itself. If not found, it uses
> kallsyms_on_each_match_symbol to search in vmlinux. If still not
> found, it calls module_kallsyms_on_each_symbol with modname set to
> NULL to search across all ko modules. The reason for not searching
> across all ko modules from the start is to avoid issues with
> duplicate symbol names.

No, your patch would break things. What are you trying to achieve? Is it 
motivated by a failure or an issue that you met? Could you share it, 
please? There may be a bug somewhere but it is difficult to judge without 
data.

> Reviewed-by: zhangchun <zhang.chunA@h3c.com>
> Reviewed-by: wangshijie <wang.shijie@h3c.com>

Drop these tags next time, please. The review happens here in the open.

> Signed-off-by: luhao <lu.haoA@h3c.com>
> ---
>  kernel/livepatch/core.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 28d15ba58a26..9c587cc4896b 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -167,9 +167,14 @@ static int klp_find_object_symbol(const char *objname, const char *name,
>                 .pos = sympos,
>         };
> 
> -       if (objname)
> +       if (objname) {
>                 module_kallsyms_on_each_symbol(objname, klp_find_callback, &args);
> -       else
> +
> +               if (args.addr == 0)
> +                       kallsyms_on_each_match_symbol(klp_match_callback, name, &args);
> +               if (args.addr == 0)
> +                       module_kallsyms_on_each_symbol(NULL, klp_find_callback, &args);
> +       } else
>                 kallsyms_on_each_match_symbol(klp_match_callback, name, &args);
> 
>         /*
> --
> 2.51.0
> 
> -------------------------------------------------------------------------------------------------------------------------------------
> ????????????????????????????????????????????????????????????????????????????????
> ??????????????????????????????????????????????????????????????????????????????????????????
> ??????????????????????????????????????????????????????????????
> This e-mail and its attachments contain confidential information from New H3C, which is intended only for the person or entity whose address is listed above.
> Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited.
> If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!

I think that you do not want the above disclaimer when you submit a patch 
to an open source project. Could you fix your email client, please?

Regards
Miroslav