linux-kernel - Re: [PATCH bpf-next v2 1/2] bpf: Add show_fdinfo for uprobe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aE_KX66K-8yrSPtS@krava>
Date: Mon, 16 Jun 2025 09:40:15 +0200
From: Jiri Olsa <olsajiri@...il.com>
To: Tao Chen <chen.dylane@...ux.dev>
Cc: song@...nel.org, kpsingh@...nel.org, mattbobrowski@...gle.com,
	ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
	martin.lau@...ux.dev, eddyz87@...il.com, yonghong.song@...ux.dev,
	john.fastabend@...il.com, sdf@...ichev.me, haoluo@...gle.com,
	rostedt@...dmis.org, mhiramat@...nel.org,
	mathieu.desnoyers@...icios.com, bpf@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 1/2] bpf: Add show_fdinfo for uprobe_multi

On Sun, Jun 15, 2025 at 11:05:13PM +0800, Tao Chen wrote:
> Show uprobe_multi link info with fdinfo, the info as follows:
> 
> link_type:	uprobe_multi
> link_id:	9
> prog_tag:	e729f789e34a8eca
> prog_id:	58
> type:	uprobe_multi
> uprobe_cnt:	3
> pid:	0
> path:	/home/dylane/bpf/tools/testing/selftests/bpf/test_progs
> offset:	0xa69ed7
> ref_ctr_offset:	0x0
> cookie:	3
> offset:	0xa69ee2
> ref_ctr_offset:	0x0
> cookie:	1
> offset:	0xa69eed
> ref_ctr_offset:	0x0
> cookie:	2

hi,
does this need to be 'tag: value' format ? bpftool uses:

        offset             ref_ctr_offset     cookies
        0xe558             0x0                0x0
        0x2574e            0x0                0x0
        0x6c393            0x0                0x0

which might be more readable, or at least extra line after each uprobe?
also using spaces instead of tabs  to align the values might help

same for kprobe_multi, otherwise looks lgtm

thanks,
jirka


> 
> Signed-off-by: Tao Chen <chen.dylane@...ux.dev>
> ---
>  kernel/trace/bpf_trace.c | 48 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> Change list:
>   v1 -> v2:
>     - replace 'func_cnt' with 'uprobe_cnt'.(Andrii)
>     - print func name is more readable and security for kprobe_multi.(Alexei)
>   v1:
>   https://lore.kernel.org/bpf/20250612115556.295103-1-chen.dylane@linux.dev
> 
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 24b94870b50..9a8ca8a8e2b 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -3157,10 +3157,58 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link,
>  	return err;
>  }
>  
> +#ifdef CONFIG_PROC_FS
> +static void bpf_uprobe_multi_show_fdinfo(const struct bpf_link *link,
> +					 struct seq_file *seq)
> +{
> +	struct bpf_uprobe_multi_link *umulti_link;
> +	char *p, *buf;
> +
> +	umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
> +
> +	buf = kmalloc(PATH_MAX, GFP_KERNEL);
> +	if (!buf)
> +		return;
> +
> +	p = d_path(&umulti_link->path, buf, PATH_MAX);
> +	if (IS_ERR(p)) {
> +		kfree(buf);
> +		return;
> +	}
> +
> +	seq_printf(seq,
> +		   "type:\t%s\n"
> +		   "uprobe_cnt:\t%u\n"
> +		   "pid:\t%u\n"
> +		   "path:\t%s\n",
> +		   umulti_link->flags == BPF_F_UPROBE_MULTI_RETURN ?
> +					 "uretprobe_multi" : "uprobe_multi",
> +		   umulti_link->cnt,
> +		   umulti_link->task ? task_pid_nr_ns(umulti_link->task,
> +			   task_active_pid_ns(current)) : 0,
> +		   p);
> +
> +	for (int i = 0; i < umulti_link->cnt; i++) {
> +		seq_printf(seq,
> +			   "offset:\t%#llx\n"
> +			   "ref_ctr_offset:\t%#lx\n"
> +			   "cookie:\t%llu\n",
> +			   umulti_link->uprobes[i].offset,
> +			   umulti_link->uprobes[i].ref_ctr_offset,
> +			   umulti_link->uprobes[i].cookie);
> +	}
> +
> +	kfree(buf);
> +}
> +#endif
> +
>  static const struct bpf_link_ops bpf_uprobe_multi_link_lops = {
>  	.release = bpf_uprobe_multi_link_release,
>  	.dealloc_deferred = bpf_uprobe_multi_link_dealloc,
>  	.fill_link_info = bpf_uprobe_multi_link_fill_link_info,
> +#ifdef CONFIG_PROC_FS
> +	.show_fdinfo = bpf_uprobe_multi_show_fdinfo,
> +#endif
>  };
>  
>  static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> -- 
> 2.48.1
>