[PATCH] livepatch: Improve the accuracy of symbol search

[PATCH] livepatch: Improve the accuracy of symbol search

[luhao]

module_kallsyms_on_each_symbol, when the input parameter modname is not
 empty, only searches for symbols within the current module. When
patching a kernel object (ko), if the patched function calls
functions from vmlinux or other ko modules, symbol lookup may fail.

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.

Reviewed-by: zhangchun <zhang.chunA@h3c.com>
Reviewed-by: wangshijie <wang.shijie@h3c.com>
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!

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

[Miroslav Benes]

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