On Thu, Sep 08, 2022 at 09:35:51PM +0900, Masami Hiramatsu wrote: > 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, thanks for review, I'll rebase and new version jirka > > > 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>
