| Message ID | 20230628115329.248450-7-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bpf: Support ->fill_link_info for kprobe_multi and perf_event links | expand |
On 6/28/23 1:53 PM, Yafang Shao wrote: > Since different symbols can share the same name, it is insufficient to only > expose the symbol name. It is essential to also expose the symbol address > so that users can accurately identify which one is being probed. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > --- > kernel/trace/trace_kprobe.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c > index e4554dbfd113..17e17298e894 100644 > --- a/kernel/trace/trace_kprobe.c > +++ b/kernel/trace/trace_kprobe.c > @@ -1547,15 +1547,15 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type, > if (tk->symbol) { > *symbol = tk->symbol; > *probe_offset = tk->rp.kp.offset; > - *probe_addr = 0; > } else { > *symbol = NULL; > *probe_offset = 0; > - if (kallsyms_show_value(current_cred())) > - *probe_addr = (unsigned long)tk->rp.kp.addr; > - else > - *probe_addr = 0; > } > + > + if (kallsyms_show_value(current_cred())) > + *probe_addr = (unsigned long)tk->rp.kp.addr; > + else > + *probe_addr = 0; > return 0; Can't this be simplified further? If tk->symbol is NULL we assign NULL anyway: diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c index 1b3fa7b854aa..bf2872ca5aaf 100644 --- a/kernel/trace/trace_kprobe.c +++ b/kernel/trace/trace_kprobe.c @@ -1544,15 +1544,10 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type, *fd_type = trace_kprobe_is_return(tk) ? BPF_FD_TYPE_KRETPROBE : BPF_FD_TYPE_KPROBE; - if (tk->symbol) { - *symbol = tk->symbol; - *probe_offset = tk->rp.kp.offset; - *probe_addr = 0; - } else { - *symbol = NULL; - *probe_offset = 0; - *probe_addr = (unsigned long)tk->rp.kp.addr; - } + *probe_offset = tk->rp.kp.offset; + *probe_addr = kallsyms_show_value(current_cred()) ? + (unsigned long)tk->rp.kp.addr : 0; + *symbol = tk->symbol; return 0; } #endif /* CONFIG_PERF_EVENTS */
On Wed, Jul 5, 2023 at 4:26 PM Daniel Borkmann <daniel@iogearbox.net> wrote: > > On 6/28/23 1:53 PM, Yafang Shao wrote: > > Since different symbols can share the same name, it is insufficient to only > > expose the symbol name. It is essential to also expose the symbol address > > so that users can accurately identify which one is being probed. > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > --- > > kernel/trace/trace_kprobe.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c > > index e4554dbfd113..17e17298e894 100644 > > --- a/kernel/trace/trace_kprobe.c > > +++ b/kernel/trace/trace_kprobe.c > > @@ -1547,15 +1547,15 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type, > > if (tk->symbol) { > > *symbol = tk->symbol; > > *probe_offset = tk->rp.kp.offset; > > - *probe_addr = 0; > > } else { > > *symbol = NULL; > > *probe_offset = 0; > > - if (kallsyms_show_value(current_cred())) > > - *probe_addr = (unsigned long)tk->rp.kp.addr; > > - else > > - *probe_addr = 0; > > } > > + > > + if (kallsyms_show_value(current_cred())) > > + *probe_addr = (unsigned long)tk->rp.kp.addr; > > + else > > + *probe_addr = 0; > > return 0; > > Can't this be simplified further? If tk->symbol is NULL we assign NULL anyway: Agree. Thanks. > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c > index 1b3fa7b854aa..bf2872ca5aaf 100644 > --- a/kernel/trace/trace_kprobe.c > +++ b/kernel/trace/trace_kprobe.c > @@ -1544,15 +1544,10 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type, > > *fd_type = trace_kprobe_is_return(tk) ? BPF_FD_TYPE_KRETPROBE > : BPF_FD_TYPE_KPROBE; > - if (tk->symbol) { > - *symbol = tk->symbol; > - *probe_offset = tk->rp.kp.offset; > - *probe_addr = 0; > - } else { > - *symbol = NULL; > - *probe_offset = 0; > - *probe_addr = (unsigned long)tk->rp.kp.addr; > - } > + *probe_offset = tk->rp.kp.offset; > + *probe_addr = kallsyms_show_value(current_cred()) ? > + (unsigned long)tk->rp.kp.addr : 0; > + *symbol = tk->symbol; > return 0; > } > #endif /* CONFIG_PERF_EVENTS */ >
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c index e4554dbfd113..17e17298e894 100644 --- a/kernel/trace/trace_kprobe.c +++ b/kernel/trace/trace_kprobe.c @@ -1547,15 +1547,15 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type, if (tk->symbol) { *symbol = tk->symbol; *probe_offset = tk->rp.kp.offset; - *probe_addr = 0; } else { *symbol = NULL; *probe_offset = 0; - if (kallsyms_show_value(current_cred())) - *probe_addr = (unsigned long)tk->rp.kp.addr; - else - *probe_addr = 0; } + + if (kallsyms_show_value(current_cred())) + *probe_addr = (unsigned long)tk->rp.kp.addr; + else + *probe_addr = 0; return 0; } #endif /* CONFIG_PERF_EVENTS */
Since different symbols can share the same name, it is insufficient to only expose the symbol name. It is essential to also expose the symbol address so that users can accurately identify which one is being probed. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- kernel/trace/trace_kprobe.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)