Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Jun 12, 2023 at 03:16:06PM +0000, Yafang Shao wrote:

SNIP

>  
>  /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 80c9ec0..fe354d5 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3303,9 +3303,133 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
>  	kfree(perf_link);
>  }
>  
> +static int bpf_perf_link_fill_name(const struct perf_event *event,
> +				   char __user *uname, u32 ulen,
> +				   u64 *probe_offset, u64 *probe_addr,
> +				   u32 *fd_type)
> +{

this function name sounds misleading, it does query all the link data
plus copying the name.. seems like this should be renamed and separated


> +	const char *buf;
> +	u32 prog_id;
> +	size_t len;
> +	int err;
> +
> +	if (!ulen ^ !uname)
> +		return -EINVAL;
> +	if (!uname)
> +		return 0;
> +
> +	err = bpf_get_perf_event_info(event, &prog_id, fd_type, &buf,
> +				      probe_offset, probe_addr);
> +	if (err)
> +		return err;
> +
> +	len = strlen(buf);
> +	if (buf) {
> +		err = bpf_copy_to_user(uname, buf, ulen, len);
> +		if (err)
> +			return err;
> +	} else {
> +		char zero = '\0';
> +
> +		if (put_user(zero, uname))
> +			return -EFAULT;
> +	}
> +	return 0;
> +}
> +
> +static int bpf_perf_link_fill_probe(const struct perf_event *event,
> +				    struct bpf_link_info *info)
> +{
> +	char __user *uname;
> +	u64 addr, offset;
> +	u32 ulen, type;
> +	int err;
> +
> +#ifdef CONFIG_KPROBE_EVENTS

this will break compilation when CONFIG_KPROBE_EVENTS or CONFIG_UPROBE_EVENTS
options are not defined

jirka

> +	if (event->tp_event->flags & TRACE_EVENT_FL_KPROBE) {
> +		uname = u64_to_user_ptr(info->kprobe.func_name);
> +		ulen = info->kprobe.name_len;
> +		info->perf_link_type = BPF_PERF_LINK_KPROBE;
> +		err = bpf_perf_link_fill_name(event, uname, ulen, &offset,
> +					      &addr, &type);
> +		if (err)
> +			return err;
> +
> +		info->kprobe.offset = offset;
> +		if (type == BPF_FD_TYPE_KRETPROBE)
> +			info->kprobe.flags = 1;
> +		if (!kallsyms_show_value(current_cred()))
> +			return 0;
> +		info->kprobe.addr = addr;
> +		return 0;
> +	}
> +#endif
> +
> +#ifdef CONFIG_UPROBE_EVENTS
> +	if (event->tp_event->flags & TRACE_EVENT_FL_UPROBE) {
> +		uname = u64_to_user_ptr(info->uprobe.file_name);
> +		ulen = info->uprobe.name_len;
> +		info->perf_link_type = BPF_PERF_LINK_UPROBE;
> +		err = bpf_perf_link_fill_name(event, uname, ulen, &offset,
> +					      &addr, &type);
> +		if (err)
> +			return err;
> +
> +		info->uprobe.offset = offset;
> +		if (type == BPF_FD_TYPE_URETPROBE)
> +			info->uprobe.flags = 1;
> +		return 0;
> +	}
> +#endif
> +
> +	return -EOPNOTSUPP;
> +}
> +

SNIP