Re: [PATCH bpf-next 8/8] bpftool: Display cookie for kprobe multi link

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, Jan 18, 2024 at 05:51:17PM +0000, Quentin Monnet wrote:

SNIP

> >  static __u64 *u64_to_arr(__u64 val)
> > @@ -675,8 +706,8 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
> >  
> >  static void show_kprobe_multi_plain(struct bpf_link_info *info)
> >  {
> > +	struct addr_cookie *data;
> >  	__u32 i, j = 0;
> > -	__u64 *addrs;
> >  
> >  	if (!info->kprobe_multi.count)
> >  		return;
> > @@ -688,8 +719,11 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
> >  	printf("func_cnt %u  ", info->kprobe_multi.count);
> >  	if (info->kprobe_multi.missed)
> >  		printf("missed %llu  ", info->kprobe_multi.missed);
> > -	addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> > -	qsort(addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
> > +	data = get_addr_cookie_array(u64_to_ptr(info->kprobe_multi.addrs),
> > +				     u64_to_ptr(info->kprobe_multi.cookies),
> > +				     info->kprobe_multi.count);
> > +	if (!data)
> > +		return;
> >  
> >  	/* Load it once for all. */
> >  	if (!dd.sym_count)
> > @@ -697,12 +731,12 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
> >  	if (!dd.sym_count)
> >  		return;
> 
> Don't we need to free(data) if we return here?

good catch! I guess I got distracted by show_kprobe_multi_plain being
similar to show_kprobe_multi_json, which does not check dd.sym_count
and does not return, which it should :-\ I'll include that fix as well

thanks,
jirka

> 
> >  
> > -	printf("\n\t%-16s %s", "addr", "func [module]");
> > +	printf("\n\t%-16s %-16s %s", "addr", "cookie", "func [module]");
> >  	for (i = 0; i < dd.sym_count; i++) {
> > -		if (dd.sym_mapping[i].address != addrs[j])
> > +		if (dd.sym_mapping[i].address != data[j].addr)
> >  			continue;
> > -		printf("\n\t%016lx %s",
> > -		       dd.sym_mapping[i].address, dd.sym_mapping[i].name);
> > +		printf("\n\t%016lx %-16llx %s",
> > +		       dd.sym_mapping[i].address, data[j].cookie, dd.sym_mapping[i].name);
> >  		if (dd.sym_mapping[i].module[0] != '\0')
> >  			printf(" [%s]  ", dd.sym_mapping[i].module);
> >  		else
> > @@ -711,6 +745,7 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
> >  		if (j++ == info->kprobe_multi.count)
> >  			break;
> >  	}
> > +	free(data);
> >  }
> >  
> >  static void show_uprobe_multi_plain(struct bpf_link_info *info)
> > @@ -966,6 +1001,14 @@ static int do_show_link(int fd)
> >  				return -ENOMEM;
> >  			}
> >  			info.kprobe_multi.addrs = ptr_to_u64(addrs);
> > +			cookies = calloc(count, sizeof(__u64));
> > +			if (!cookies) {
> > +				p_err("mem alloc failed");
> > +				free(addrs);
> > +				close(fd);
> > +				return -ENOMEM;
> > +			}
> > +			info.kprobe_multi.cookies = ptr_to_u64(cookies);
> >  			goto again;
> >  		}
> >  	}
>