On Fri, 7 Jun 2024, Song Liu wrote: > Hi Miroslav, > > Thanks for reviewing the patch! > > On Fri, Jun 7, 2024 at 6:06 AM Miroslav Benes <mbenes@xxxxxxx> wrote: > > > > Hi, > > > > On Tue, 4 Jun 2024, Song Liu wrote: > > > > > With CONFIG_LTO_CLANG, the compiler may postfix symbols with .llvm.<hash> > > > to avoid symbol duplication. scripts/kallsyms.c sorted the symbols > > > without these postfixes. The default symbol lookup also removes these > > > postfixes before comparing symbols. > > > > > > On the other hand, livepatch need to look up symbols with the full names. > > > However, calling kallsyms_on_each_match_symbol with full name (with the > > > postfix) cannot find the symbol(s). As a result, we cannot livepatch > > > kernel functions with .llvm.<hash> postfix or kernel functions that use > > > relocation information to symbols with .llvm.<hash> postfixes. > > > > > > Fix this by calling kallsyms_on_each_match_symbol without the postfix; > > > and then match the full name (with postfix) in klp_match_callback. > > > > > > Signed-off-by: Song Liu <song@xxxxxxxxxx> > > > --- > > > include/linux/kallsyms.h | 13 +++++++++++++ > > > kernel/kallsyms.c | 21 ++++++++++++++++----- > > > kernel/livepatch/core.c | 32 +++++++++++++++++++++++++++++++- > > > 3 files changed, 60 insertions(+), 6 deletions(-) > > > > I do not like much that something which seems to be kallsyms-internal is > > leaked out. You need to export cleanup_symbol_name() and there is now a > > lot of code outside. I would feel much more comfortable if it is all > > hidden from kallsyms users and kept there. Would it be possible? > > I think it is possible. Currently, kallsyms_on_each_match_symbol matches > symbols without the postfix. We can add a variation or a parameter, so > that it matches the full name with post fix. I think it might be better. Luis, what is your take on this? > > Moreover, isn't there a similar problem for ftrace, kprobes, ebpf,...? > > Yes, there is a similar problem with tracing use cases. But the requirements > are not the same: > > For livepatch, we have to point to the exact symbol we want to patch or > relocation to. We have sympos API defined to differentiate different symbols > with the same name. Yes. In fact, sympos may be used to solve even this problem. The user would disregard .llvm.<hash> suffix and they are suddenly in the same situation which sympos aims to solve. I will not argue with you if say it is cumbersome. > For tracing, some discrepancy is acceptable. AFAICT, there isn't an API > similar to sympos yet. Also, we can play some tricks with tracing. For > example, we can use "uniq symbol + offset" to point a kprobe to one of > the duplicated symbols. If I am not mistaken, there was a patch set to address this. Luis might remember more. Regards, Miroslav
