Re: [PATCH bpf-next 3/3] bpftool: Add tracing_multi link info output

[Jiri Olsa <olsajiri@gmail.com>]

On Sun, Jun 21, 2026 at 08:56:27PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Repeated parsing of `/proc/kallsyms` in an O(N) loop when symbol loading fails.
> --
> 
> commit 0f11f8e705a95b6aa483a05374ffb9b22bcb78c0
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpftool: Add tracing_multi link info output
> 
> This commit adds bpftool support to show tracing_multi link details for both
> plain and JSON outputs.
> 
> > diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> > index bdcd717b0348f..58dadee984a07 100644
> > --- a/tools/bpf/bpftool/link.c
> > +++ b/tools/bpf/bpftool/link.c
> > @@ -377,6 +377,31 @@ static __u64 *u64_to_arr(__u64 val)
> >  	return (__u64 *) u64_to_ptr(val);
> >  }
> >  
> > +static __u32 *u64_to_u32_arr(__u64 val)
> > +{
> > +	return (__u32 *)u64_to_ptr(val);
> > +}
> > +
> > +static struct kernel_sym *find_kernel_sym_by_addr(__u64 addr, bool is_ibt_enabled)
> > +{
> > +	struct kernel_sym *sym;
> > +
> > +	if (!addr)
> > +		return NULL;
> > +
> > +	/* Load it once for all. */
> > +	if (!dd.sym_count)
> > +		kernel_syms_load(&dd);
> > +	if (!dd.sym_count)
> > +		return NULL;
> > +
> > +	sym = kernel_syms_search(&dd, addr);
> > +	if (!sym && is_ibt_enabled && addr >= 4)
> > +		sym = kernel_syms_search(&dd, addr - 4);
> > +
> > +	return sym;
> > +}
> 
> [ ... ]
> 
> > @@ -403,6 +428,43 @@ show_uprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
> >  	jsonw_end_array(json_wtr);
> >  }
> >  
> > +static void
> > +show_tracing_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
> > +{
> > +	bool is_ibt_enabled = is_x86_ibt_enabled();
> > +	__u32 i;
> > +
> > +	show_link_attach_type_json(info->tracing_multi.attach_type, wtr);
> > +	jsonw_uint_field(wtr, "func_cnt", info->tracing_multi.count);
> > +	jsonw_uint_field(wtr, "obj_id", info->tracing_multi.obj_id);
> > +	jsonw_name(wtr, "funcs");
> > +
> > +	jsonw_start_array(wtr);
> > +
> > +	for (i = 0; i < info->tracing_multi.count; i++) {
> > +		__u64 addr = u64_to_arr(info->tracing_multi.addrs)[i];
> > +		struct kernel_sym *sym;
> > +
> > +		sym = find_kernel_sym_by_addr(addr, is_ibt_enabled);
> 
> [Severity: Medium]
> If kernel_syms_load() fails (for example, due to kernel.kptr_restrict or
> a lack of read permissions for /proc/kallsyms), dd.sym_count remains 0.
> 
> Since find_kernel_sym_by_addr() checks !dd.sym_count to decide whether to load
> symbols, won't this cause /proc/kallsyms to be repeatedly opened and parsed on
> every iteration of this loop if the initial load fails?
> 
> This loop occurs here in show_tracing_multi_json() as well as in
> show_tracing_multi_plain(). For tracing_multi links with potentially thousands
> of targets, could this result in a significant performance hang for bpftool?

ok, I think we can make sure the parsing happens just once with
the change below, will do that in next version

jirka


---
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 58dadee984a0..db780ee39820 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -389,12 +389,6 @@ static struct kernel_sym *find_kernel_sym_by_addr(__u64 addr, bool is_ibt_enable
 	if (!addr)
 		return NULL;
 
-	/* Load it once for all. */
-	if (!dd.sym_count)
-		kernel_syms_load(&dd);
-	if (!dd.sym_count)
-		return NULL;
-
 	sym = kernel_syms_search(&dd, addr);
 	if (!sym && is_ibt_enabled && addr >= 4)
 		sym = kernel_syms_search(&dd, addr - 4);
@@ -431,9 +425,13 @@ show_uprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 static void
 show_tracing_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 {
-	bool is_ibt_enabled = is_x86_ibt_enabled();
+	bool is_ibt_enabled = is_x86_ibt_enabled(), show_symbol;
 	__u32 i;
 
+	if (!dd.sym_count)
+		kernel_syms_load(&dd);
+	show_symbol = !!dd.sym_count;
+
 	show_link_attach_type_json(info->tracing_multi.attach_type, wtr);
 	jsonw_uint_field(wtr, "func_cnt", info->tracing_multi.count);
 	jsonw_uint_field(wtr, "obj_id", info->tracing_multi.obj_id);
@@ -445,7 +443,7 @@ show_tracing_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 		__u64 addr = u64_to_arr(info->tracing_multi.addrs)[i];
 		struct kernel_sym *sym;
 
-		sym = find_kernel_sym_by_addr(addr, is_ibt_enabled);
+		sym = show_symbol ? find_kernel_sym_by_addr(addr, is_ibt_enabled) : NULL;
 
 		jsonw_start_object(wtr);
 		jsonw_uint_field(wtr, "id", u64_to_u32_arr(info->tracing_multi.ids)[i]);
@@ -900,12 +898,16 @@ static void show_uprobe_multi_plain(struct bpf_link_info *info)
 
 static void show_tracing_multi_plain(struct bpf_link_info *info)
 {
-	bool is_ibt_enabled = is_x86_ibt_enabled();
+	bool is_ibt_enabled = is_x86_ibt_enabled(), show_symbol;
 	__u32 i;
 
 	if (!info->tracing_multi.count)
 		return;
 
+	if (!dd.sym_count)
+		kernel_syms_load(&dd);
+	show_symbol = !!dd.sym_count;
+
 	printf("\n\t");
 	show_link_attach_type_plain(info->tracing_multi.attach_type);
 	printf("obj_id %u  ", info->tracing_multi.obj_id);
@@ -917,7 +919,7 @@ static void show_tracing_multi_plain(struct bpf_link_info *info)
 		__u64 addr = u64_to_arr(info->tracing_multi.addrs)[i];
 		struct kernel_sym *sym;
 
-		sym = find_kernel_sym_by_addr(addr, is_ibt_enabled);
+		sym = show_symbol ? find_kernel_sym_by_addr(addr, is_ibt_enabled) : NULL;
 
 		printf("\n\t%-16u %016llx %-16llx",
 		       u64_to_u32_arr(info->tracing_multi.ids)[i],