On Thu, 11 Aug 2022 11:15:22 +0200 Jiri Olsa <jolsa@xxxxxxxxxx> wrote: > Keeping the resolved 'addr' in kallsyms_callback, instead of taking > ftrace_location value, because we depend on symbol address in the > cookie related code. > > With CONFIG_X86_KERNEL_IBT option the ftrace_location value differs > from symbol address, which screwes the symbol address cookies matching. > > There are 2 users of this function: > - bpf_kprobe_multi_link_attach > for which this fix is for > > - get_ftrace_locations > which is used by register_fprobe_syms > > this function needs to get symbols resolved to addresses, > but does not need 'ftrace location addresses' at this point > there's another ftrace location translation in the path done > by ftrace_set_filter_ips call: > > register_fprobe_syms > addrs = get_ftrace_locations > > register_fprobe_ips(addrs) > ... > ftrace_set_filter_ips > ... > __ftrace_match_addr > ip = ftrace_location(ip); > ... > Yes, this looks OK for fprobe. I confirmed above. Reviewed-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> One concern was that the caller might expect that the address must be ftrace_location(), but as far as I can read the function document, there is no such description. * ftrace_lookup_symbols - Lookup addresses for array of symbols ... * This function looks up addresses for array of symbols provided in * @syms array (must be alphabetically sorted) and stores them in * @addrs array, which needs to be big enough to store at least @cnt * addresses. So this change is OK. Thank you, > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx> > --- > kernel/trace/ftrace.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index bc921a3f7ea8..8a8c90d1a387 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -8268,8 +8268,7 @@ static int kallsyms_callback(void *data, const char *name, > if (args->addrs[idx]) > return 0; > > - addr = ftrace_location(addr); > - if (!addr) > + if (!ftrace_location(addr)) > return 0; > > args->addrs[idx] = addr; > -- > 2.37.1 > -- Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
