On Fri, Jun 3, 2022 at 3:16 AM Jiri Olsa [off-list ref] wrote:

On Thu, Jun 02, 2022 at 03:52:03PM -0700, Andrii Nakryiko wrote:

quoted

On Fri, May 27, 2022 at 1:56 PM Jiri Olsa [off-list ref] wrote:

quoted

We want to store the resolved address on the same index as
the symbol string, because that's the user (bpf kprobe link)
code assumption.

Also making sure we don't store duplicates that might be
present in kallsyms.

Fixes: bed0d9a50dac ("ftrace: Add ftrace_lookup_symbols function")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/trace/ftrace.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 674add0aafb3..00d0ba6397ed 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c

@@ -7984,15 +7984,23 @@ static int kallsyms_callback(void *data, const char *name,
                             struct module *mod, unsigned long addr)
 {
        struct kallsyms_data *args = data;
+       const char **sym;
+       int idx;

-       if (!bsearch(&name, args->syms, args->cnt, sizeof(*args->syms), symbols_cmp))
+       sym = bsearch(&name, args->syms, args->cnt, sizeof(*args->syms), symbols_cmp);
+       if (!sym)
+               return 0;
+
+       idx = sym - args->syms;
+       if (args->addrs[idx])

if we have duplicated symbols we won't increment args->found here,
right? So we won't stop early. But we also don't want to increment
args->found here because we use it to check that we don't have
duplicates (in addition to making sure we resolved all the unique
symbols), right?

So I wonder if in this situation should we return some error code to
signify that we encountered symbol duplicate?

hum, this callback is called for each kallsyms symbol and there
are duplicates in /proc/kallsyms.. so even if we have just single
copy of such symbol in args->syms, bsearch will find this single
symbol for all the duplicates in /proc/kallsyms and we will endup
in here.. and it's still fine, we should continue

ah, ok, duplicate kallsyms entries, right, never mind then

jirka

quoted

quoted

                return 0;

        addr = ftrace_location(addr);
        if (!addr)
                return 0;

-       args->addrs[args->found++] = addr;
+       args->addrs[idx] = addr;
+       args->found++;
        return args->found == args->cnt ? 1 : 0;
 }

@@ -8017,6 +8025,7 @@ int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *a
        struct kallsyms_data args;
        int err;

+       memset(addrs, 0x0, sizeof(*addrs) * cnt);
        args.addrs = addrs;
        args.syms = sorted_syms;
        args.cnt = cnt;
--

2.35.3