[PATCH v6 bpf-next 00/11] bpf: Support ->fill_link_info for kprobe_multi and perf_event links

[PATCH v6 bpf-next 00/11] bpf: Support ->fill_link_info for kprobe_multi and perf_event links

[Yafang Shao]

This patchset enhances the usability of kprobe_multi program by introducing
support for ->fill_link_info. This allows users to easily determine the
probed functions associated with a kprobe_multi program. While
`bpftool perf show` already provides information about functions probed by
perf_event programs, supporting ->fill_link_info ensures consistent access
to this information across all bpf links.

In addition, this patch extends support to generic perf events, which are
currently not covered by `bpftool perf show`. While userspace is exposed to
only the perf type and config, other attributes such as sample_period and
sample_freq are disregarded.

To ensure accurate identification of probed functions, it is preferable to
expose the address directly rather than relying solely on the symbol name.
However, this implementation respects the kptr_restrict setting and avoids
exposing the address if it is not permitted.

v5->v6:
- From Andrii
  - if ucount is too less, copy ucount items and return -E2BIG 
  - zero out kmulti_link->cnt elements if it is not permitted by kptr
  - avoid leaking information when ucount is greater than kmulti_link->cnt
  - drop the flags, and add BPF_PERF_EVENT_[UK]RETPROBE 
- From Quentin
  - use jsonw_null instead when we have no module name
  - add explanation on perf_type_name in the commit log
  - avoid the unnecessary out lable 

v4->v5:
- Print "func [module]" in the kprobe_multi header (Andrii)
- Remove MAX_BPF_PERF_EVENT_TYPE (Alexei)
- Add padding field for future reuse (Yonghong)

v3->v4:
- From Quentin
  - Rename MODULE_NAME_LEN to MODULE_MAX_NAME
  - Convert retprobe to boolean for json output
  - Trim the square brackets around module names for json output
  - Move perf names into link.c
  - Use a generic helper to get perf names
  - Show address before func name, for consistency
  - Use switch-case instead of if-else
  - Increase the buff len to PATH_MAX
  - Move macros to the top of the file
- From Andrii
  - kprobe_multi flags should always be returned
  - Keep it single line if it fits in under 100 characters
  - Change the output format when showing kprobe_multi
  - Imporve the format of perf_event names
  - Rename struct perf_link to struct perf_event, and change the names of
    the enum consequently
- From Yonghong
  - Avoid disallowing extensions for all structs in the big union
- From Jiri
  - Add flags to bpf_kprobe_multi_link
  - Report kprobe_multi selftests errors
  - Rename bpf_perf_link_fill_name and make it a separate patch
  - Avoid breaking compilation when CONFIG_KPROBE_EVENTS or
    CONFIG_UPROBE_EVENTS options are not defined

v2->v3:
- Expose flags instead of retporbe (Andrii)
- Simplify the check on kmulti_link->cnt (Andrii)
- Use kallsyms_show_value() instead (Andrii)
- Show also the module name for kprobe_multi (Andrii)
- Add new enum bpf_perf_link_type (Andrii)
- Move perf event names into bpftool (Andrii, Quentin, Jiri)
- Keep perf event names in sync with perf tools (Jiri) 

v1->v2:
- Fix sparse warning (Stanislav, lkp@intel.com)
- Fix BPF CI build error
- Reuse kernel_syms_load() (Alexei)
- Print 'name' instead of 'func' (Alexei)
- Show whether the probe is retprobe or not (Andrii)
- Add comment for the meaning of perf_event name (Andrii)
- Add support for generic perf event
- Adhere to the kptr_restrict setting

RFC->v1:
- Use a single copy_to_user() instead (Jiri)
- Show also the symbol name in bpftool (Quentin, Alexei)
- Use calloc() instead of malloc() in bpftool (Quentin)
- Avoid having conditional entries in the JSON output (Quentin)
- Drop ->show_fdinfo (Alexei)
- Use __u64 instead of __aligned_u64 for the field addr (Alexei)
- Avoid the contradiction in perf_event name length (Alexei) 
- Address a build warning reported by kernel test robot <lkp@intel.com>

Yafang Shao (11):
  bpf: Support ->fill_link_info for kprobe_multi
  bpftool: Dump the kernel symbol's module name
  bpftool: Show kprobe_multi link info
  bpf: Protect probed address based on kptr_restrict setting
  bpf: Clear the probe_addr for uprobe
  bpf: Expose symbol's respective address
  bpf: Add a common helper bpf_copy_to_user()
  bpf: Add bpf_perf_link_fill_common()
  bpf: Support ->fill_link_info for perf_event
  bpftool: Add perf event names
  bpftool: Show perf link info

 include/uapi/linux/bpf.h          |  40 +++
 kernel/bpf/syscall.c              | 185 ++++++++++++-
 kernel/trace/bpf_trace.c          |  41 ++-
 kernel/trace/trace_kprobe.c       |   7 +-
 tools/bpf/bpftool/link.c          | 428 +++++++++++++++++++++++++++++-
 tools/bpf/bpftool/xlated_dumper.c |   6 +-
 tools/bpf/bpftool/xlated_dumper.h |   2 +
 tools/include/uapi/linux/bpf.h    |  40 +++
 8 files changed, 729 insertions(+), 20 deletions(-)

-- 
2.39.3

[PATCH v6 bpf-next 01/11] bpf: Support ->fill_link_info for kprobe_multi

[Yafang Shao]

With the addition of support for fill_link_info to the kprobe_multi link,
users will gain the ability to inspect it conveniently using the
`bpftool link show`. This enhancement provides valuable information to the
user, including the count of probed functions and their respective
addresses. It's important to note that if the kptr_restrict setting is not
permitted, the probed address will not be exposed, ensuring security.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/uapi/linux/bpf.h       |  5 +++++
 kernel/trace/bpf_trace.c       | 37 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h |  5 +++++
 3 files changed, 47 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 60a9d59beeab..512ba3ba2ed3 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -6439,6 +6439,11 @@ struct bpf_link_info {
 			__s32 priority;
 			__u32 flags;
 		} netfilter;
+		struct {
+			__aligned_u64 addrs; /* in/out: addresses buffer ptr */
+			__u32 count;
+			__u32 flags;
+		} kprobe_multi;
 	};
 } __attribute__((aligned(8)));
 
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 03b7f6b8e4f0..1f9f78e1992f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2469,6 +2469,7 @@ struct bpf_kprobe_multi_link {
 	u32 cnt;
 	u32 mods_cnt;
 	struct module **mods;
+	u32 flags;
 };
 
 struct bpf_kprobe_multi_run_ctx {
@@ -2558,9 +2559,44 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
 	kfree(kmulti_link);
 }
 
+static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
+						struct bpf_link_info *info)
+{
+	u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
+	struct bpf_kprobe_multi_link *kmulti_link;
+	u32 ucount = info->kprobe_multi.count;
+	int err = 0, i;
+
+	if (!uaddrs ^ !ucount)
+		return -EINVAL;
+
+	kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
+	info->kprobe_multi.count = kmulti_link->cnt;
+	info->kprobe_multi.flags = kmulti_link->flags;
+
+	if (!uaddrs)
+		return 0;
+	if (ucount < kmulti_link->cnt)
+		err = -E2BIG;
+	else
+		ucount = kmulti_link->cnt;
+
+	if (kallsyms_show_value(current_cred())) {
+		if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64)))
+			return -EFAULT;
+	} else {
+		for (i = 0; i < ucount; i++) {
+			if (put_user(0, uaddrs + i))
+				return -EFAULT;
+		}
+	}
+	return err;
+}
+
 static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
 	.release = bpf_kprobe_multi_link_release,
 	.dealloc = bpf_kprobe_multi_link_dealloc,
+	.fill_link_info = bpf_kprobe_multi_link_fill_link_info,
 };
 
 static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv)
@@ -2872,6 +2908,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	link->addrs = addrs;
 	link->cookies = cookies;
 	link->cnt = cnt;
+	link->flags = flags;
 
 	if (cookies) {
 		/*
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 60a9d59beeab..512ba3ba2ed3 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6439,6 +6439,11 @@ struct bpf_link_info {
 			__s32 priority;
 			__u32 flags;
 		} netfilter;
+		struct {
+			__aligned_u64 addrs; /* in/out: addresses buffer ptr */
+			__u32 count;
+			__u32 flags;
+		} kprobe_multi;
 	};
 } __attribute__((aligned(8)));
 
-- 
2.39.3

[PATCH v6 bpf-next 02/11] bpftool: Dump the kernel symbol's module name

[Yafang Shao]

If the kernel symbol is in a module, we will dump the module name as
well. The square brackets around the module name are trimmed.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/bpf/bpftool/xlated_dumper.c | 6 +++++-
 tools/bpf/bpftool/xlated_dumper.h | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
index da608e10c843..567f56dfd9f1 100644
--- a/tools/bpf/bpftool/xlated_dumper.c
+++ b/tools/bpf/bpftool/xlated_dumper.c
@@ -46,7 +46,11 @@ void kernel_syms_load(struct dump_data *dd)
 		}
 		dd->sym_mapping = tmp;
 		sym = &dd->sym_mapping[dd->sym_count];
-		if (sscanf(buff, "%p %*c %s", &address, sym->name) != 2)
+
+		/* module is optional */
+		sym->module[0] = '\0';
+		/* trim the square brackets around the module name */
+		if (sscanf(buff, "%p %*c %s [%[^]]s", &address, sym->name, sym->module) < 2)
 			continue;
 		sym->address = (unsigned long)address;
 		if (!strcmp(sym->name, "__bpf_call_base")) {
diff --git a/tools/bpf/bpftool/xlated_dumper.h b/tools/bpf/bpftool/xlated_dumper.h
index 9a946377b0e6..db3ba0671501 100644
--- a/tools/bpf/bpftool/xlated_dumper.h
+++ b/tools/bpf/bpftool/xlated_dumper.h
@@ -5,12 +5,14 @@
 #define __BPF_TOOL_XLATED_DUMPER_H
 
 #define SYM_MAX_NAME	256
+#define MODULE_MAX_NAME	64
 
 struct bpf_prog_linfo;
 
 struct kernel_sym {
 	unsigned long address;
 	char name[SYM_MAX_NAME];
+	char module[MODULE_MAX_NAME];
 };
 
 struct dump_data {
-- 
2.39.3

[PATCH v6 bpf-next 03/11] bpftool: Show kprobe_multi link info

[Yafang Shao]

Show the already expose kprobe_multi link info in bpftool. The result as
follows,

$ tools/bpf/bpftool/bpftool link show
91: kprobe_multi  prog 244
        kprobe.multi  func_cnt 7
        addr             func [module]
        ffffffff98c44f20 schedule_timeout_interruptible
        ffffffff98c44f60 schedule_timeout_killable
        ffffffff98c44fa0 schedule_timeout_uninterruptible
        ffffffff98c44fe0 schedule_timeout_idle
        ffffffffc075b8d0 xfs_trans_get_efd [xfs]
        ffffffffc0768a10 xfs_trans_get_buf_map [xfs]
        ffffffffc076c320 xfs_trans_get_dqtrx [xfs]
        pids kprobe_multi(188367)
92: kprobe_multi  prog 244
        kretprobe.multi  func_cnt 7
        addr             func [module]
        ffffffff98c44f20 schedule_timeout_interruptible
        ffffffff98c44f60 schedule_timeout_killable
        ffffffff98c44fa0 schedule_timeout_uninterruptible
        ffffffff98c44fe0 schedule_timeout_idle
        ffffffffc075b8d0 xfs_trans_get_efd [xfs]
        ffffffffc0768a10 xfs_trans_get_buf_map [xfs]
        ffffffffc076c320 xfs_trans_get_dqtrx [xfs]
        pids kprobe_multi(188367)

$ tools/bpf/bpftool/bpftool link show -j
[{"id":91,"type":"kprobe_multi","prog_id":244,"retprobe":false,"func_cnt":7,"funcs":[{"addr":18446744071977586464,"func":"schedule_timeout_interruptible","module":null},{"addr":18446744071977586528,"func":"schedule_timeout_killable","module":null},{"addr":18446744071977586592,"func":"schedule_timeout_uninterruptible","module":null},{"addr":18446744071977586656,"func":"schedule_timeout_idle","module":null},{"addr":18446744072643524816,"func":"xfs_trans_get_efd","module":"xfs"},{"addr":18446744072643578384,"func":"xfs_trans_get_buf_map","module":"xfs"},{"addr":18446744072643592992,"func":"xfs_trans_get_dqtrx","module":"xfs"}],"pids":[{"pid":188367,"comm":"kprobe_multi"}]},{"id":92,"type":"kprobe_multi","prog_id":244,"retprobe":true,"func_cnt":7,"funcs":[{"addr":18446744071977586464,"func":"schedule_timeout_interruptible","module":null},{"addr":18446744071977586528,"func":"schedule_timeout_killable","module":null},{"addr":18446744071977586592,"func":"schedule_timeout_uninterruptible","module":null},{"addr":18446744071977586656,"func":"schedule_timeout_idle","module":null},{"addr":18446744072643524816,"func":"xfs_trans_get_efd","module":"xfs"},{"addr":18446744072643578384,"func":"xfs_trans_get_buf_map","module":"xfs"},{"addr":18446744072643592992,"func":"xfs_trans_get_dqtrx","module":"xfs"}],"pids":[{"pid":188367,"comm":"kprobe_multi"}]}]

When kptr_restrict is 2, the result is,

$ tools/bpf/bpftool/bpftool link show
91: kprobe_multi  prog 244
        kprobe.multi  func_cnt 7
92: kprobe_multi  prog 244
        kretprobe.multi  func_cnt 7

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/bpf/bpftool/link.c | 114 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 113 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 2d786072ed0d..34fa17cd9d2a 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -14,8 +14,10 @@
 
 #include "json_writer.h"
 #include "main.h"
+#include "xlated_dumper.h"
 
 static struct hashmap *link_table;
+static struct dump_data dd = {};
 
 static int link_parse_fd(int *argc, char ***argv)
 {
@@ -166,6 +168,50 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
 	return err;
 }
 
+static int cmp_u64(const void *A, const void *B)
+{
+	const __u64 *a = A, *b = B;
+
+	return *a - *b;
+}
+
+static void
+show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
+{
+	__u32 i, j = 0;
+	__u64 *addrs;
+
+	jsonw_bool_field(json_wtr, "retprobe",
+			 info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
+	jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
+	jsonw_name(json_wtr, "funcs");
+	jsonw_start_array(json_wtr);
+	addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
+	qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
+
+	/* Load it once for all. */
+	if (!dd.sym_count)
+		kernel_syms_load(&dd);
+	for (i = 0; i < dd.sym_count; i++) {
+		if (dd.sym_mapping[i].address != addrs[j])
+			continue;
+		jsonw_start_object(json_wtr);
+		jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
+		jsonw_string_field(json_wtr, "func", dd.sym_mapping[i].name);
+		/* Print null if it is vmlinux */
+		if (dd.sym_mapping[i].module[0] == '\0') {
+			jsonw_name(json_wtr, "module");
+			jsonw_null(json_wtr);
+		} else {
+			jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
+		}
+		jsonw_end_object(json_wtr);
+		if (j++ == info->kprobe_multi.count)
+			break;
+	}
+	jsonw_end_array(json_wtr);
+}
+
 static int show_link_close_json(int fd, struct bpf_link_info *info)
 {
 	struct bpf_prog_info prog_info;
@@ -218,6 +264,9 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
 		jsonw_uint_field(json_wtr, "map_id",
 				 info->struct_ops.map_id);
 		break;
+	case BPF_LINK_TYPE_KPROBE_MULTI:
+		show_kprobe_multi_json(info, json_wtr);
+		break;
 	default:
 		break;
 	}
@@ -351,6 +400,44 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
 		printf(" flags 0x%x", info->netfilter.flags);
 }
 
+static void show_kprobe_multi_plain(struct bpf_link_info *info)
+{
+	__u32 i, j = 0;
+	__u64 *addrs;
+
+	if (!info->kprobe_multi.count)
+		return;
+
+	if (info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN)
+		printf("\n\tkretprobe.multi  ");
+	else
+		printf("\n\tkprobe.multi  ");
+	printf("func_cnt %u  ", info->kprobe_multi.count);
+	addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
+	qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
+
+	/* Load it once for all. */
+	if (!dd.sym_count)
+		kernel_syms_load(&dd);
+	if (!dd.sym_count)
+		return;
+
+	printf("\n\t%-16s %s", "addr", "func [module]");
+	for (i = 0; i < dd.sym_count; i++) {
+		if (dd.sym_mapping[i].address != addrs[j])
+			continue;
+		printf("\n\t%016lx %s",
+		       dd.sym_mapping[i].address, dd.sym_mapping[i].name);
+		if (dd.sym_mapping[i].module[0] != '\0')
+			printf(" [%s]  ", dd.sym_mapping[i].module);
+		else
+			printf("  ");
+
+		if (j++ == info->kprobe_multi.count)
+			break;
+	}
+}
+
 static int show_link_close_plain(int fd, struct bpf_link_info *info)
 {
 	struct bpf_prog_info prog_info;
@@ -396,6 +483,9 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
 	case BPF_LINK_TYPE_NETFILTER:
 		netfilter_dump_plain(info);
 		break;
+	case BPF_LINK_TYPE_KPROBE_MULTI:
+		show_kprobe_multi_plain(info);
+		break;
 	default:
 		break;
 	}
@@ -417,7 +507,9 @@ static int do_show_link(int fd)
 {
 	struct bpf_link_info info;
 	__u32 len = sizeof(info);
+	__u64 *addrs = NULL;
 	char buf[256];
+	int count;
 	int err;
 
 	memset(&info, 0, sizeof(info));
@@ -441,12 +533,28 @@ static int do_show_link(int fd)
 		info.iter.target_name_len = sizeof(buf);
 		goto again;
 	}
+	if (info.type == BPF_LINK_TYPE_KPROBE_MULTI &&
+	    !info.kprobe_multi.addrs) {
+		count = info.kprobe_multi.count;
+		if (count) {
+			addrs = calloc(count, sizeof(__u64));
+			if (!addrs) {
+				p_err("mem alloc failed");
+				close(fd);
+				return -1;
+			}
+			info.kprobe_multi.addrs = (unsigned long)addrs;
+			goto again;
+		}
+	}
 
 	if (json_output)
 		show_link_close_json(fd, &info);
 	else
 		show_link_close_plain(fd, &info);
 
+	if (addrs)
+		free(addrs);
 	close(fd);
 	return 0;
 }
@@ -471,7 +579,8 @@ static int do_show(int argc, char **argv)
 		fd = link_parse_fd(&argc, &argv);
 		if (fd < 0)
 			return fd;
-		return do_show_link(fd);
+		do_show_link(fd);
+		goto out;
 	}
 
 	if (argc)
@@ -510,6 +619,9 @@ static int do_show(int argc, char **argv)
 	if (show_pinned)
 		delete_pinned_obj_table(link_table);
 
+out:
+	if (dd.sym_count)
+		kernel_syms_destroy(&dd);
 	return errno == ENOENT ? 0 : -1;
 }
 
-- 
2.39.3

[PATCH v6 bpf-next 04/11] bpf: Protect probed address based on kptr_restrict setting

[Yafang Shao]

The probed address can be accessed by userspace through querying the task
file descriptor (fd). However, it is crucial to adhere to the kptr_restrict
setting and refrain from exposing the address if it is not permitted.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/trace/trace_kprobe.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 59cda19a9033..e4554dbfd113 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1551,7 +1551,10 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type,
 	} else {
 		*symbol = NULL;
 		*probe_offset = 0;
-		*probe_addr = (unsigned long)tk->rp.kp.addr;
+		if (kallsyms_show_value(current_cred()))
+			*probe_addr = (unsigned long)tk->rp.kp.addr;
+		else
+			*probe_addr = 0;
 	}
 	return 0;
 }
-- 
2.39.3

[PATCH v6 bpf-next 05/11] bpf: Clear the probe_addr for uprobe

[Yafang Shao]

To avoid returning uninitialized or random values when querying the file
descriptor (fd) and accessing probe_addr, it is necessary to clear the
variable prior to its use.

Fixes: 41bdc4b40ed6 ("bpf: introduce bpf subcommand BPF_TASK_FD_QUERY")
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
 kernel/trace/bpf_trace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1f9f78e1992f..ac9958907a7c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2382,10 +2382,12 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
 						  event->attr.type == PERF_TYPE_TRACEPOINT);
 #endif
 #ifdef CONFIG_UPROBE_EVENTS
-		if (flags & TRACE_EVENT_FL_UPROBE)
+		if (flags & TRACE_EVENT_FL_UPROBE) {
 			err = bpf_get_uprobe_info(event, fd_type, buf,
 						  probe_offset,
 						  event->attr.type == PERF_TYPE_TRACEPOINT);
+			*probe_addr = 0x0;
+		}
 #endif
 	}
 
-- 
2.39.3

[PATCH v6 bpf-next 06/11] bpf: Expose symbol's respective address

[Yafang Shao]

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;
 }
 #endif	/* CONFIG_PERF_EVENTS */
-- 
2.39.3

[PATCH v6 bpf-next 07/11] bpf: Add a common helper bpf_copy_to_user()

[Yafang Shao]

Add a common helper bpf_copy_to_user(), which will be used at multiple
places.
No functional change.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/bpf/syscall.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index a2aef900519c..4aa6e5776a04 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3295,6 +3295,25 @@ static void bpf_raw_tp_link_show_fdinfo(const struct bpf_link *link,
 		   raw_tp_link->btp->tp->name);
 }
 
+static int bpf_copy_to_user(char __user *ubuf, const char *buf, u32 ulen,
+			    u32 len)
+{
+	if (ulen >= len + 1) {
+		if (copy_to_user(ubuf, buf, len + 1))
+			return -EFAULT;
+	} else {
+		char zero = '\0';
+
+		if (copy_to_user(ubuf, buf, ulen - 1))
+			return -EFAULT;
+		if (put_user(zero, ubuf + ulen - 1))
+			return -EFAULT;
+		return -ENOSPC;
+	}
+
+	return 0;
+}
+
 static int bpf_raw_tp_link_fill_link_info(const struct bpf_link *link,
 					  struct bpf_link_info *info)
 {
@@ -3313,20 +3332,7 @@ static int bpf_raw_tp_link_fill_link_info(const struct bpf_link *link,
 	if (!ubuf)
 		return 0;
 
-	if (ulen >= tp_len + 1) {
-		if (copy_to_user(ubuf, tp_name, tp_len + 1))
-			return -EFAULT;
-	} else {
-		char zero = '\0';
-
-		if (copy_to_user(ubuf, tp_name, ulen - 1))
-			return -EFAULT;
-		if (put_user(zero, ubuf + ulen - 1))
-			return -EFAULT;
-		return -ENOSPC;
-	}
-
-	return 0;
+	return bpf_copy_to_user(ubuf, tp_name, ulen, tp_len);
 }
 
 static const struct bpf_link_ops bpf_raw_tp_link_lops = {
-- 
2.39.3

[PATCH v6 bpf-next 08/11] bpf: Add bpf_perf_link_fill_common()

[Yafang Shao]

Add a new helper bpf_perf_link_fill_common(), which will be used by
perf_link based tracepoint, kprobe and uprobe.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/bpf/syscall.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4aa6e5776a04..72de91beabbc 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3364,6 +3364,40 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
 	kfree(perf_link);
 }
 
+static int bpf_perf_link_fill_common(const struct perf_event *event,
+				     char __user *uname, u32 ulen,
+				     u64 *probe_offset, u64 *probe_addr,
+				     u32 *fd_type)
+{
+	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 const struct bpf_link_ops bpf_perf_link_lops = {
 	.release = bpf_perf_link_release,
 	.dealloc = bpf_perf_link_dealloc,
-- 
2.39.3

[PATCH v6 bpf-next 09/11] bpf: Support ->fill_link_info for perf_event

[Yafang Shao]

By introducing support for ->fill_link_info to the perf_event link, users
gain the ability to inspect it using `bpftool link show`. While the current
approach involves accessing this information via `bpftool perf show`,
consolidating link information for all link types in one place offers
greater convenience. Additionally, this patch extends support to the
generic perf event, which is not currently accommodated by
`bpftool perf show`. While only the perf type and config are exposed to
userspace, other attributes such as sample_period and sample_freq are
ignored. It's important to note that if kptr_restrict is not permitted, the
probed address will not be exposed, maintaining security measures.

A new enum bpf_perf_event_type is introduced to help the user understand
which struct is relevant.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/uapi/linux/bpf.h       |  35 ++++++++++
 kernel/bpf/syscall.c           | 117 +++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h |  35 ++++++++++
 3 files changed, 187 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 512ba3ba2ed3..7efe51672c15 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1057,6 +1057,16 @@ enum bpf_link_type {
 	MAX_BPF_LINK_TYPE,
 };
 
+enum bpf_perf_event_type {
+	BPF_PERF_EVENT_UNSPEC = 0,
+	BPF_PERF_EVENT_UPROBE = 1,
+	BPF_PERF_EVENT_URETPROBE = 2,
+	BPF_PERF_EVENT_KPROBE = 3,
+	BPF_PERF_EVENT_KRETPROBE = 4,
+	BPF_PERF_EVENT_TRACEPOINT = 5,
+	BPF_PERF_EVENT_EVENT = 6,
+};
+
 /* cgroup-bpf attach flags used in BPF_PROG_ATTACH command
  *
  * NONE(default): No further bpf programs allowed in the subtree.
@@ -6444,6 +6454,31 @@ struct bpf_link_info {
 			__u32 count;
 			__u32 flags;
 		} kprobe_multi;
+		struct {
+			__u32 type; /* enum bpf_perf_event_type */
+			__u32 :32;
+			union {
+				struct {
+					__aligned_u64 file_name; /* in/out */
+					__u32 name_len;
+					__u32 offset;/* offset from file_name */
+				} uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
+				struct {
+					__aligned_u64 func_name; /* in/out */
+					__u32 name_len;
+					__u32 offset;/* offset from func_name */
+					__u64 addr;
+				} kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
+				struct {
+					__aligned_u64 tp_name;   /* in/out */
+					__u32 name_len;
+				} tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
+				struct {
+					__u64 config;
+					__u32 type;
+				} event; /* BPF_PERF_EVENT_EVENT */
+			};
+		} perf_event;
 	};
 } __attribute__((aligned(8)));
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 72de91beabbc..05ff0a560f1a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3398,9 +3398,126 @@ static int bpf_perf_link_fill_common(const struct perf_event *event,
 	return 0;
 }
 
+#ifdef CONFIG_KPROBE_EVENTS
+static int bpf_perf_link_fill_kprobe(const struct perf_event *event,
+				     struct bpf_link_info *info)
+{
+	char __user *uname;
+	u64 addr, offset;
+	u32 ulen, type;
+	int err;
+
+	uname = u64_to_user_ptr(info->perf_event.kprobe.func_name);
+	ulen = info->perf_event.kprobe.name_len;
+	err = bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
+					&type);
+	if (err)
+		return err;
+	if (type == BPF_FD_TYPE_KRETPROBE)
+		info->perf_event.type = BPF_PERF_EVENT_KRETPROBE;
+	else
+		info->perf_event.type = BPF_PERF_EVENT_KPROBE;
+
+	info->perf_event.kprobe.offset = offset;
+	if (!kallsyms_show_value(current_cred()))
+		addr = 0;
+	info->perf_event.kprobe.addr = addr;
+	return 0;
+}
+#endif
+
+#ifdef CONFIG_UPROBE_EVENTS
+static int bpf_perf_link_fill_uprobe(const struct perf_event *event,
+				     struct bpf_link_info *info)
+{
+	char __user *uname;
+	u64 addr, offset;
+	u32 ulen, type;
+	int err;
+
+	uname = u64_to_user_ptr(info->perf_event.uprobe.file_name);
+	ulen = info->perf_event.uprobe.name_len;
+	err = bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
+					&type);
+	if (err)
+		return err;
+
+	if (type == BPF_FD_TYPE_URETPROBE)
+		info->perf_event.type = BPF_PERF_EVENT_URETPROBE;
+	else
+		info->perf_event.type = BPF_PERF_EVENT_UPROBE;
+	info->perf_event.uprobe.offset = offset;
+	return 0;
+}
+#endif
+
+static int bpf_perf_link_fill_probe(const struct perf_event *event,
+				    struct bpf_link_info *info)
+{
+#ifdef CONFIG_KPROBE_EVENTS
+	if (event->tp_event->flags & TRACE_EVENT_FL_KPROBE)
+		return bpf_perf_link_fill_kprobe(event, info);
+#endif
+#ifdef CONFIG_UPROBE_EVENTS
+	if (event->tp_event->flags & TRACE_EVENT_FL_UPROBE)
+		return bpf_perf_link_fill_uprobe(event, info);
+#endif
+	return -EOPNOTSUPP;
+}
+
+static int bpf_perf_link_fill_tracepoint(const struct perf_event *event,
+					 struct bpf_link_info *info)
+{
+	char __user *uname;
+	u64 addr, offset;
+	u32 ulen, type;
+
+	uname = u64_to_user_ptr(info->perf_event.tracepoint.tp_name);
+	ulen = info->perf_event.tracepoint.name_len;
+	info->perf_event.type = BPF_PERF_EVENT_TRACEPOINT;
+	return bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
+					 &type);
+}
+
+static int bpf_perf_link_fill_perf_event(const struct perf_event *event,
+					 struct bpf_link_info *info)
+{
+	info->perf_event.event.type = event->attr.type;
+	info->perf_event.event.config = event->attr.config;
+	info->perf_event.type = BPF_PERF_EVENT_EVENT;
+	return 0;
+}
+
+static int bpf_perf_link_fill_link_info(const struct bpf_link *link,
+					struct bpf_link_info *info)
+{
+	struct bpf_perf_link *perf_link;
+	const struct perf_event *event;
+
+	perf_link = container_of(link, struct bpf_perf_link, link);
+	event = perf_get_event(perf_link->perf_file);
+	if (IS_ERR(event))
+		return PTR_ERR(event);
+
+	if (!event->prog)
+		return -EINVAL;
+
+	switch (event->prog->type) {
+	case BPF_PROG_TYPE_PERF_EVENT:
+		return bpf_perf_link_fill_perf_event(event, info);
+	case BPF_PROG_TYPE_TRACEPOINT:
+		return bpf_perf_link_fill_tracepoint(event, info);
+	case BPF_PROG_TYPE_KPROBE:
+		return bpf_perf_link_fill_probe(event, info);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static const struct bpf_link_ops bpf_perf_link_lops = {
 	.release = bpf_perf_link_release,
 	.dealloc = bpf_perf_link_dealloc,
+	.fill_link_info = bpf_perf_link_fill_link_info,
 };
 
 static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 512ba3ba2ed3..7efe51672c15 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1057,6 +1057,16 @@ enum bpf_link_type {
 	MAX_BPF_LINK_TYPE,
 };
 
+enum bpf_perf_event_type {
+	BPF_PERF_EVENT_UNSPEC = 0,
+	BPF_PERF_EVENT_UPROBE = 1,
+	BPF_PERF_EVENT_URETPROBE = 2,
+	BPF_PERF_EVENT_KPROBE = 3,
+	BPF_PERF_EVENT_KRETPROBE = 4,
+	BPF_PERF_EVENT_TRACEPOINT = 5,
+	BPF_PERF_EVENT_EVENT = 6,
+};
+
 /* cgroup-bpf attach flags used in BPF_PROG_ATTACH command
  *
  * NONE(default): No further bpf programs allowed in the subtree.
@@ -6444,6 +6454,31 @@ struct bpf_link_info {
 			__u32 count;
 			__u32 flags;
 		} kprobe_multi;
+		struct {
+			__u32 type; /* enum bpf_perf_event_type */
+			__u32 :32;
+			union {
+				struct {
+					__aligned_u64 file_name; /* in/out */
+					__u32 name_len;
+					__u32 offset;/* offset from file_name */
+				} uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
+				struct {
+					__aligned_u64 func_name; /* in/out */
+					__u32 name_len;
+					__u32 offset;/* offset from func_name */
+					__u64 addr;
+				} kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
+				struct {
+					__aligned_u64 tp_name;   /* in/out */
+					__u32 name_len;
+				} tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
+				struct {
+					__u64 config;
+					__u32 type;
+				} event; /* BPF_PERF_EVENT_EVENT */
+			};
+		} perf_event;
 	};
 } __attribute__((aligned(8)));
 
-- 
2.39.3

[PATCH v6 bpf-next 10/11] bpftool: Add perf event names

[Yafang Shao]

Add new functions and macros to get perf event names. These names except
the perf_type_name are all copied from
tool/perf/util/{parse-events,evsel}.c, so that in the future we will
have a good chance to use the same code.

Suggested-by: Jiri Olsa <olsajiri@gmail.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/bpf/bpftool/link.c | 67 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 34fa17cd9d2a..ba67ea9d77fb 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -5,6 +5,7 @@
 #include <linux/err.h>
 #include <linux/netfilter.h>
 #include <linux/netfilter_arp.h>
+#include <linux/perf_event.h>
 #include <net/if.h>
 #include <stdio.h>
 #include <unistd.h>
@@ -19,6 +20,72 @@
 static struct hashmap *link_table;
 static struct dump_data dd = {};
 
+static const char *perf_type_name[PERF_TYPE_MAX] = {
+	[PERF_TYPE_HARDWARE]			= "hardware",
+	[PERF_TYPE_SOFTWARE]			= "software",
+	[PERF_TYPE_TRACEPOINT]			= "tracepoint",
+	[PERF_TYPE_HW_CACHE]			= "hw-cache",
+	[PERF_TYPE_RAW]				= "raw",
+	[PERF_TYPE_BREAKPOINT]			= "breakpoint",
+};
+
+const char *event_symbols_hw[PERF_COUNT_HW_MAX] = {
+	[PERF_COUNT_HW_CPU_CYCLES]		= "cpu-cycles",
+	[PERF_COUNT_HW_INSTRUCTIONS]		= "instructions",
+	[PERF_COUNT_HW_CACHE_REFERENCES]	= "cache-references",
+	[PERF_COUNT_HW_CACHE_MISSES]		= "cache-misses",
+	[PERF_COUNT_HW_BRANCH_INSTRUCTIONS]	= "branch-instructions",
+	[PERF_COUNT_HW_BRANCH_MISSES]		= "branch-misses",
+	[PERF_COUNT_HW_BUS_CYCLES]		= "bus-cycles",
+	[PERF_COUNT_HW_STALLED_CYCLES_FRONTEND]	= "stalled-cycles-frontend",
+	[PERF_COUNT_HW_STALLED_CYCLES_BACKEND]	= "stalled-cycles-backend",
+	[PERF_COUNT_HW_REF_CPU_CYCLES]		= "ref-cycles",
+};
+
+const char *event_symbols_sw[PERF_COUNT_SW_MAX] = {
+	[PERF_COUNT_SW_CPU_CLOCK]		= "cpu-clock",
+	[PERF_COUNT_SW_TASK_CLOCK]		= "task-clock",
+	[PERF_COUNT_SW_PAGE_FAULTS]		= "page-faults",
+	[PERF_COUNT_SW_CONTEXT_SWITCHES]	= "context-switches",
+	[PERF_COUNT_SW_CPU_MIGRATIONS]		= "cpu-migrations",
+	[PERF_COUNT_SW_PAGE_FAULTS_MIN]		= "minor-faults",
+	[PERF_COUNT_SW_PAGE_FAULTS_MAJ]		= "major-faults",
+	[PERF_COUNT_SW_ALIGNMENT_FAULTS]	= "alignment-faults",
+	[PERF_COUNT_SW_EMULATION_FAULTS]	= "emulation-faults",
+	[PERF_COUNT_SW_DUMMY]			= "dummy",
+	[PERF_COUNT_SW_BPF_OUTPUT]		= "bpf-output",
+	[PERF_COUNT_SW_CGROUP_SWITCHES]		= "cgroup-switches",
+};
+
+const char *evsel__hw_cache[PERF_COUNT_HW_CACHE_MAX] = {
+	[PERF_COUNT_HW_CACHE_L1D]		= "L1-dcache",
+	[PERF_COUNT_HW_CACHE_L1I]		= "L1-icache",
+	[PERF_COUNT_HW_CACHE_LL]		= "LLC",
+	[PERF_COUNT_HW_CACHE_DTLB]		= "dTLB",
+	[PERF_COUNT_HW_CACHE_ITLB]		= "iTLB",
+	[PERF_COUNT_HW_CACHE_BPU]		= "branch",
+	[PERF_COUNT_HW_CACHE_NODE]		= "node",
+};
+
+const char *evsel__hw_cache_op[PERF_COUNT_HW_CACHE_OP_MAX] = {
+	[PERF_COUNT_HW_CACHE_OP_READ]		= "load",
+	[PERF_COUNT_HW_CACHE_OP_WRITE]		= "store",
+	[PERF_COUNT_HW_CACHE_OP_PREFETCH]	= "prefetch",
+};
+
+const char *evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX] = {
+	[PERF_COUNT_HW_CACHE_RESULT_ACCESS]	= "refs",
+	[PERF_COUNT_HW_CACHE_RESULT_MISS]	= "misses",
+};
+
+#define perf_event_name(array, id) ({			\
+	const char *event_str = NULL;			\
+							\
+	if ((id) >= 0 && (id) < ARRAY_SIZE(array))	\
+		event_str = array[id];			\
+	event_str;					\
+})
+
 static int link_parse_fd(int *argc, char ***argv)
 {
 	int fd;
-- 
2.39.3

[PATCH v6 bpf-next 11/11] bpftool: Show perf link info

[Yafang Shao]

Enhance bpftool to display comprehensive information about exposed
perf_event links, covering uprobe, kprobe, tracepoint, and generic perf
event. The resulting output will include the following details:

$ tools/bpf/bpftool/bpftool link show
3: perf_event  prog 14
        event software:cpu-clock
        bpf_cookie 0
        pids perf_event(19483)
4: perf_event  prog 14
        event hw-cache:LLC-load-misses
        bpf_cookie 0
        pids perf_event(19483)
5: perf_event  prog 14
        event hardware:cpu-cycles
        bpf_cookie 0
        pids perf_event(19483)
6: perf_event  prog 19
        tracepoint sched_switch
        bpf_cookie 0
        pids tracepoint(20947)
7: perf_event  prog 26
        uprobe /home/dev/waken/bpf/uprobe/a.out+0x1338
        bpf_cookie 0
        pids uprobe(21973)
8: perf_event  prog 27
        uretprobe /home/dev/waken/bpf/uprobe/a.out+0x1338
        bpf_cookie 0
        pids uprobe(21973)
10: perf_event  prog 43
        kprobe ffffffffb70a9660 kernel_clone
        bpf_cookie 0
        pids kprobe(35275)
11: perf_event  prog 41
        kretprobe ffffffffb70a9660 kernel_clone
        bpf_cookie 0
        pids kprobe(35275)

$ tools/bpf/bpftool/bpftool link show -j
[{"id":3,"type":"perf_event","prog_id":14,"event_type":"software","event_config":"cpu-clock","bpf_cookie":0,"pids":[{"pid":19483,"comm":"perf_event"}]},{"id":4,"type":"perf_event","prog_id":14,"event_type":"hw-cache","event_config":"LLC-load-misses","bpf_cookie":0,"pids":[{"pid":19483,"comm":"perf_event"}]},{"id":5,"type":"perf_event","prog_id":14,"event_type":"hardware","event_config":"cpu-cycles","bpf_cookie":0,"pids":[{"pid":19483,"comm":"perf_event"}]},{"id":6,"type":"perf_event","prog_id":19,"tracepoint":"sched_switch","bpf_cookie":0,"pids":[{"pid":20947,"comm":"tracepoint"}]},{"id":7,"type":"perf_event","prog_id":26,"retprobe":false,"file":"/home/dev/waken/bpf/uprobe/a.out","offset":4920,"bpf_cookie":0,"pids":[{"pid":21973,"comm":"uprobe"}]},{"id":8,"type":"perf_event","prog_id":27,"retprobe":true,"file":"/home/dev/waken/bpf/uprobe/a.out","offset":4920,"bpf_cookie":0,"pids":[{"pid":21973,"comm":"uprobe"}]},{"id":10,"type":"perf_event","prog_id":43,"retprobe":false,"addr":18446744072485508704,"func":"kernel_clone","offset":0,"bpf_cookie":0,"pids":[{"pid":35275,"comm":"kprobe"}]},{"id":11,"type":"perf_event","prog_id":41,"retprobe":true,"addr":18446744072485508704,"func":"kernel_clone","offset":0,"bpf_cookie":0,"pids":[{"pid":35275,"comm":"kprobe"}]}]

For generic perf events, the displayed information in bpftool is limited to
the type and configuration, while other attributes such as sample_period,
sample_freq, etc., are not included.

The kernel function address won't be exposed if it is not permitted by
kptr_restrict. The result as follows when kptr_restrict is 2.

$ tools/bpf/bpftool/bpftool link show
3: perf_event  prog 14
        event software:cpu-clock
4: perf_event  prog 14
        event hw-cache:LLC-load-misses
5: perf_event  prog 14
        event hardware:cpu-cycles
6: perf_event  prog 19
        tracepoint sched_switch
7: perf_event  prog 26
        uprobe /home/dev/waken/bpf/uprobe/a.out+0x1338
8: perf_event  prog 27
        uretprobe /home/dev/waken/bpf/uprobe/a.out+0x1338
10: perf_event  prog 43
        kprobe kernel_clone
11: perf_event  prog 41
        kretprobe kernel_clone

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 tools/bpf/bpftool/link.c | 247 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 246 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index ba67ea9d77fb..7c4c9fc45f63 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -17,6 +17,8 @@
 #include "main.h"
 #include "xlated_dumper.h"
 
+#define PERF_HW_CACHE_LEN 128
+
 static struct hashmap *link_table;
 static struct dump_data dd = {};
 
@@ -279,6 +281,110 @@ show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 	jsonw_end_array(json_wtr);
 }
 
+static void
+show_perf_event_kprobe_json(struct bpf_link_info *info, json_writer_t *wtr)
+{
+	jsonw_bool_field(wtr, "retprobe", info->perf_event.type == BPF_PERF_EVENT_KRETPROBE);
+	jsonw_uint_field(wtr, "addr", info->perf_event.kprobe.addr);
+	jsonw_string_field(wtr, "func",
+			   u64_to_ptr(info->perf_event.kprobe.func_name));
+	jsonw_uint_field(wtr, "offset", info->perf_event.kprobe.offset);
+}
+
+static void
+show_perf_event_uprobe_json(struct bpf_link_info *info, json_writer_t *wtr)
+{
+	jsonw_bool_field(wtr, "retprobe", info->perf_event.type == BPF_PERF_EVENT_URETPROBE);
+	jsonw_string_field(wtr, "file",
+			   u64_to_ptr(info->perf_event.uprobe.file_name));
+	jsonw_uint_field(wtr, "offset", info->perf_event.uprobe.offset);
+}
+
+static void
+show_perf_event_tracepoint_json(struct bpf_link_info *info, json_writer_t *wtr)
+{
+	jsonw_string_field(wtr, "tracepoint",
+			   u64_to_ptr(info->perf_event.tracepoint.tp_name));
+}
+
+static char *perf_config_hw_cache_str(__u64 config)
+{
+	const char *hw_cache, *result, *op;
+	char *str = malloc(PERF_HW_CACHE_LEN);
+
+	if (!str) {
+		p_err("mem alloc failed");
+		return NULL;
+	}
+
+	hw_cache = perf_event_name(evsel__hw_cache, config & 0xff);
+	if (hw_cache)
+		snprintf(str, PERF_HW_CACHE_LEN, "%s-", hw_cache);
+	else
+		snprintf(str, PERF_HW_CACHE_LEN, "%lld-", config & 0xff);
+
+	op = perf_event_name(evsel__hw_cache_op, (config >> 8) & 0xff);
+	if (op)
+		snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
+			 "%s-", op);
+	else
+		snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
+			 "%lld-", (config >> 8) & 0xff);
+
+	result = perf_event_name(evsel__hw_cache_result, config >> 16);
+	if (result)
+		snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
+			 "%s", result);
+	else
+		snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
+			 "%lld", config >> 16);
+	return str;
+}
+
+static const char *perf_config_str(__u32 type, __u64 config)
+{
+	const char *perf_config;
+
+	switch (type) {
+	case PERF_TYPE_HARDWARE:
+		perf_config = perf_event_name(event_symbols_hw, config);
+		break;
+	case PERF_TYPE_SOFTWARE:
+		perf_config = perf_event_name(event_symbols_sw, config);
+		break;
+	case PERF_TYPE_HW_CACHE:
+		perf_config = perf_config_hw_cache_str(config);
+		break;
+	default:
+		perf_config = NULL;
+		break;
+	}
+	return perf_config;
+}
+
+static void
+show_perf_event_event_json(struct bpf_link_info *info, json_writer_t *wtr)
+{
+	__u64 config = info->perf_event.event.config;
+	__u32 type = info->perf_event.event.type;
+	const char *perf_type, *perf_config;
+
+	perf_type = perf_event_name(perf_type_name, type);
+	if (perf_type)
+		jsonw_string_field(wtr, "event_type", perf_type);
+	else
+		jsonw_uint_field(wtr, "event_type", type);
+
+	perf_config = perf_config_str(type, config);
+	if (perf_config)
+		jsonw_string_field(wtr, "event_config", perf_config);
+	else
+		jsonw_uint_field(wtr, "event_config", config);
+
+	if (type == PERF_TYPE_HW_CACHE && perf_config)
+		free((void *)perf_config);
+}
+
 static int show_link_close_json(int fd, struct bpf_link_info *info)
 {
 	struct bpf_prog_info prog_info;
@@ -334,6 +440,26 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
 	case BPF_LINK_TYPE_KPROBE_MULTI:
 		show_kprobe_multi_json(info, json_wtr);
 		break;
+	case BPF_LINK_TYPE_PERF_EVENT:
+		switch (info->perf_event.type) {
+		case BPF_PERF_EVENT_EVENT:
+			show_perf_event_event_json(info, json_wtr);
+			break;
+		case BPF_PERF_EVENT_TRACEPOINT:
+			show_perf_event_tracepoint_json(info, json_wtr);
+			break;
+		case BPF_PERF_EVENT_KPROBE:
+		case BPF_PERF_EVENT_KRETPROBE:
+			show_perf_event_kprobe_json(info, json_wtr);
+			break;
+		case BPF_PERF_EVENT_UPROBE:
+		case BPF_PERF_EVENT_URETPROBE:
+			show_perf_event_uprobe_json(info, json_wtr);
+			break;
+		default:
+			break;
+		}
+		break;
 	default:
 		break;
 	}
@@ -505,6 +631,75 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 	}
 }
 
+static void show_perf_event_kprobe_plain(struct bpf_link_info *info)
+{
+	const char *buf;
+
+	buf = (const char *)u64_to_ptr(info->perf_event.kprobe.func_name);
+	if (buf[0] == '\0' && !info->perf_event.kprobe.addr)
+		return;
+
+	if (info->perf_event.type == BPF_PERF_EVENT_KRETPROBE)
+		printf("\n\tkretprobe ");
+	else
+		printf("\n\tkprobe ");
+	if (info->perf_event.kprobe.addr)
+		printf("%llx ", info->perf_event.kprobe.addr);
+	printf("%s", buf);
+	if (info->perf_event.kprobe.offset)
+		printf("+%#x", info->perf_event.kprobe.offset);
+	printf("  ");
+}
+
+static void show_perf_event_uprobe_plain(struct bpf_link_info *info)
+{
+	const char *buf;
+
+	buf = (const char *)u64_to_ptr(info->perf_event.uprobe.file_name);
+	if (buf[0] == '\0')
+		return;
+
+	if (info->perf_event.type == BPF_PERF_EVENT_URETPROBE)
+		printf("\n\turetprobe ");
+	else
+		printf("\n\tuprobe ");
+	printf("%s+%#x  ", buf, info->perf_event.uprobe.offset);
+}
+
+static void show_perf_event_tracepoint_plain(struct bpf_link_info *info)
+{
+	const char *buf;
+
+	buf = (const char *)u64_to_ptr(info->perf_event.tracepoint.tp_name);
+	if (buf[0] == '\0')
+		return;
+
+	printf("\n\ttracepoint %s  ", buf);
+}
+
+static void show_perf_event_event_plain(struct bpf_link_info *info)
+{
+	__u64 config = info->perf_event.event.config;
+	__u32 type = info->perf_event.event.type;
+	const char *perf_type, *perf_config;
+
+	printf("\n\tevent ");
+	perf_type = perf_event_name(perf_type_name, type);
+	if (perf_type)
+		printf("%s:", perf_type);
+	else
+		printf("%u :", type);
+
+	perf_config = perf_config_str(type, config);
+	if (perf_config)
+		printf("%s  ", perf_config);
+	else
+		printf("%llu  ", config);
+
+	if (type == PERF_TYPE_HW_CACHE && perf_config)
+		free((void *)perf_config);
+}
+
 static int show_link_close_plain(int fd, struct bpf_link_info *info)
 {
 	struct bpf_prog_info prog_info;
@@ -553,6 +748,26 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
 	case BPF_LINK_TYPE_KPROBE_MULTI:
 		show_kprobe_multi_plain(info);
 		break;
+	case BPF_LINK_TYPE_PERF_EVENT:
+		switch (info->perf_event.type) {
+		case BPF_PERF_EVENT_EVENT:
+			show_perf_event_event_plain(info);
+			break;
+		case BPF_PERF_EVENT_TRACEPOINT:
+			show_perf_event_tracepoint_plain(info);
+			break;
+		case BPF_PERF_EVENT_KPROBE:
+		case BPF_PERF_EVENT_KRETPROBE:
+			show_perf_event_kprobe_plain(info);
+			break;
+		case BPF_PERF_EVENT_UPROBE:
+		case BPF_PERF_EVENT_URETPROBE:
+			show_perf_event_uprobe_plain(info);
+			break;
+		default:
+			break;
+		}
+		break;
 	default:
 		break;
 	}
@@ -575,11 +790,12 @@ static int do_show_link(int fd)
 	struct bpf_link_info info;
 	__u32 len = sizeof(info);
 	__u64 *addrs = NULL;
-	char buf[256];
+	char buf[PATH_MAX];
 	int count;
 	int err;
 
 	memset(&info, 0, sizeof(info));
+	buf[0] = '\0';
 again:
 	err = bpf_link_get_info_by_fd(fd, &info, &len);
 	if (err) {
@@ -614,6 +830,35 @@ static int do_show_link(int fd)
 			goto again;
 		}
 	}
+	if (info.type == BPF_LINK_TYPE_PERF_EVENT) {
+		switch (info.perf_event.type) {
+		case BPF_PERF_EVENT_TRACEPOINT:
+			if (!info.perf_event.tracepoint.tp_name) {
+				info.perf_event.tracepoint.tp_name = (unsigned long)&buf;
+				info.perf_event.tracepoint.name_len = sizeof(buf);
+				goto again;
+			}
+			break;
+		case BPF_PERF_EVENT_KPROBE:
+		case BPF_PERF_EVENT_KRETPROBE:
+			if (!info.perf_event.kprobe.func_name) {
+				info.perf_event.kprobe.func_name = (unsigned long)&buf;
+				info.perf_event.kprobe.name_len = sizeof(buf);
+				goto again;
+			}
+			break;
+		case BPF_PERF_EVENT_UPROBE:
+		case BPF_PERF_EVENT_URETPROBE:
+			if (!info.perf_event.uprobe.file_name) {
+				info.perf_event.uprobe.file_name = (unsigned long)&buf;
+				info.perf_event.uprobe.name_len = sizeof(buf);
+				goto again;
+			}
+			break;
+		default:
+			break;
+		}
+	}
 
 	if (json_output)
 		show_link_close_json(fd, &info);
-- 
2.39.3

Re: [PATCH v6 bpf-next 00/11] bpf: Support ->fill_link_info for kprobe_multi and perf_event links

[Jiri Olsa]

On Wed, Jun 28, 2023 at 11:53:18AM +0000, Yafang Shao wrote:
> This patchset enhances the usability of kprobe_multi program by introducing
> support for ->fill_link_info. This allows users to easily determine the
> probed functions associated with a kprobe_multi program. While
> `bpftool perf show` already provides information about functions probed by
> perf_event programs, supporting ->fill_link_info ensures consistent access
> to this information across all bpf links.
> 
> In addition, this patch extends support to generic perf events, which are
> currently not covered by `bpftool perf show`. While userspace is exposed to
> only the perf type and config, other attributes such as sample_period and
> sample_freq are disregarded.
> 
> To ensure accurate identification of probed functions, it is preferable to
> expose the address directly rather than relying solely on the symbol name.
> However, this implementation respects the kptr_restrict setting and avoids
> exposing the address if it is not permitted.
> 
> v5->v6:
> - From Andrii
>   - if ucount is too less, copy ucount items and return -E2BIG 
>   - zero out kmulti_link->cnt elements if it is not permitted by kptr
>   - avoid leaking information when ucount is greater than kmulti_link->cnt
>   - drop the flags, and add BPF_PERF_EVENT_[UK]RETPROBE 
> - From Quentin
>   - use jsonw_null instead when we have no module name
>   - add explanation on perf_type_name in the commit log
>   - avoid the unnecessary out lable 

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

Re: [PATCH v6 bpf-next 11/11] bpftool: Show perf link info

[Quentin Monnet]

2023-06-28 11:53 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> Enhance bpftool to display comprehensive information about exposed
> perf_event links, covering uprobe, kprobe, tracepoint, and generic perf
> event. The resulting output will include the following details:
> 
> $ tools/bpf/bpftool/bpftool link show
> 3: perf_event  prog 14
>         event software:cpu-clock
>         bpf_cookie 0
>         pids perf_event(19483)
> 4: perf_event  prog 14
>         event hw-cache:LLC-load-misses
>         bpf_cookie 0
>         pids perf_event(19483)
> 5: perf_event  prog 14
>         event hardware:cpu-cycles
>         bpf_cookie 0
>         pids perf_event(19483)
> 6: perf_event  prog 19
>         tracepoint sched_switch
>         bpf_cookie 0
>         pids tracepoint(20947)
> 7: perf_event  prog 26
>         uprobe /home/dev/waken/bpf/uprobe/a.out+0x1338
>         bpf_cookie 0
>         pids uprobe(21973)
> 8: perf_event  prog 27
>         uretprobe /home/dev/waken/bpf/uprobe/a.out+0x1338
>         bpf_cookie 0
>         pids uprobe(21973)
> 10: perf_event  prog 43
>         kprobe ffffffffb70a9660 kernel_clone
>         bpf_cookie 0
>         pids kprobe(35275)
> 11: perf_event  prog 41
>         kretprobe ffffffffb70a9660 kernel_clone
>         bpf_cookie 0
>         pids kprobe(35275)
> 
> $ tools/bpf/bpftool/bpftool link show -j
> [{"id":3,"type":"perf_event","prog_id":14,"event_type":"software","event_config":"cpu-clock","bpf_cookie":0,"pids":[{"pid":19483,"comm":"perf_event"}]},{"id":4,"type":"perf_event","prog_id":14,"event_type":"hw-cache","event_config":"LLC-load-misses","bpf_cookie":0,"pids":[{"pid":19483,"comm":"perf_event"}]},{"id":5,"type":"perf_event","prog_id":14,"event_type":"hardware","event_config":"cpu-cycles","bpf_cookie":0,"pids":[{"pid":19483,"comm":"perf_event"}]},{"id":6,"type":"perf_event","prog_id":19,"tracepoint":"sched_switch","bpf_cookie":0,"pids":[{"pid":20947,"comm":"tracepoint"}]},{"id":7,"type":"perf_event","prog_id":26,"retprobe":false,"file":"/home/dev/waken/bpf/uprobe/a.out","offset":4920,"bpf_cookie":0,"pids":[{"pid":21973,"comm":"uprobe"}]},{"id":8,"type":"perf_event","prog_id":27,"retprobe":true,"file":"/home/dev/waken/bpf/uprobe/a.out","offset":4920,"bpf_cookie":0,"pids":[{"pid":21973,"comm":"uprobe"}]},{"id":10,"type":"perf_event","prog_id":43,"retprobe":false,"addr":18446744072485508704,"func":"kernel_clone","offset":0,"bpf_cookie":0,"pids":[{"pid":35275,"comm":"kprobe"}]},{"id":11,"type":"perf_event","prog_id":41,"retprobe":true,"addr":18446744072485508704,"func":"kernel_clone","offset":0,"bpf_cookie":0,"pids":[{"pid":35275,"comm":"kprobe"}]}]
> 
> For generic perf events, the displayed information in bpftool is limited to
> the type and configuration, while other attributes such as sample_period,
> sample_freq, etc., are not included.
> 
> The kernel function address won't be exposed if it is not permitted by
> kptr_restrict. The result as follows when kptr_restrict is 2.
> 
> $ tools/bpf/bpftool/bpftool link show
> 3: perf_event  prog 14
>         event software:cpu-clock
> 4: perf_event  prog 14
>         event hw-cache:LLC-load-misses
> 5: perf_event  prog 14
>         event hardware:cpu-cycles
> 6: perf_event  prog 19
>         tracepoint sched_switch
> 7: perf_event  prog 26
>         uprobe /home/dev/waken/bpf/uprobe/a.out+0x1338
> 8: perf_event  prog 27
>         uretprobe /home/dev/waken/bpf/uprobe/a.out+0x1338
> 10: perf_event  prog 43
>         kprobe kernel_clone
> 11: perf_event  prog 41
>         kretprobe kernel_clone
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>

Reviewed-by: Quentin Monnet <quentin@isovalent.com>

Thank you!

Re: [PATCH v6 bpf-next 02/11] bpftool: Dump the kernel symbol's module name

[Quentin Monnet]

2023-06-28 11:53 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> If the kernel symbol is in a module, we will dump the module name as
> well. The square brackets around the module name are trimmed.
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Reviewed-by: Quentin Monnet <quentin@isovalent.com>
> ---
>  tools/bpf/bpftool/xlated_dumper.c | 6 +++++-
>  tools/bpf/bpftool/xlated_dumper.h | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
> index da608e10c843..567f56dfd9f1 100644
> --- a/tools/bpf/bpftool/xlated_dumper.c
> +++ b/tools/bpf/bpftool/xlated_dumper.c
> @@ -46,7 +46,11 @@ void kernel_syms_load(struct dump_data *dd)
>  		}
>  		dd->sym_mapping = tmp;
>  		sym = &dd->sym_mapping[dd->sym_count];
> -		if (sscanf(buff, "%p %*c %s", &address, sym->name) != 2)
> +
> +		/* module is optional */
> +		sym->module[0] = '\0';
> +		/* trim the square brackets around the module name */
> +		if (sscanf(buff, "%p %*c %s [%[^]]s", &address, sym->name, sym->module) < 2)

Looking again at this patch, we should be good for parsing the module
name with the sscanf() because I don't expect a module name longer than
MODULE_MAX_NAME to show up, but I wonder what guarantee we have about
symbols names staying under SYM_MAX_NAME? Maybe we should specify the
max length to read, to remain on the safe side (or in case these limits
change in the future). But it doesn't have to be part of your set, I can
send a follow-up after that.

>  			continue;
>  		sym->address = (unsigned long)address;
>  		if (!strcmp(sym->name, "__bpf_call_base")) {
> diff --git a/tools/bpf/bpftool/xlated_dumper.h b/tools/bpf/bpftool/xlated_dumper.h
> index 9a946377b0e6..db3ba0671501 100644
> --- a/tools/bpf/bpftool/xlated_dumper.h
> +++ b/tools/bpf/bpftool/xlated_dumper.h
> @@ -5,12 +5,14 @@
>  #define __BPF_TOOL_XLATED_DUMPER_H
>  
>  #define SYM_MAX_NAME	256
> +#define MODULE_MAX_NAME	64
>  
>  struct bpf_prog_linfo;
>  
>  struct kernel_sym {
>  	unsigned long address;
>  	char name[SYM_MAX_NAME];
> +	char module[MODULE_MAX_NAME];
>  };
>  
>  struct dump_data {

Re: [PATCH v6 bpf-next 02/11] bpftool: Dump the kernel symbol's module name

[Yafang Shao]

On Thu, Jun 29, 2023 at 9:46 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2023-06-28 11:53 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> > If the kernel symbol is in a module, we will dump the module name as
> > well. The square brackets around the module name are trimmed.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Reviewed-by: Quentin Monnet <quentin@isovalent.com>
> > ---
> >  tools/bpf/bpftool/xlated_dumper.c | 6 +++++-
> >  tools/bpf/bpftool/xlated_dumper.h | 2 ++
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
> > index da608e10c843..567f56dfd9f1 100644
> > --- a/tools/bpf/bpftool/xlated_dumper.c
> > +++ b/tools/bpf/bpftool/xlated_dumper.c
> > @@ -46,7 +46,11 @@ void kernel_syms_load(struct dump_data *dd)
> >               }
> >               dd->sym_mapping = tmp;
> >               sym = &dd->sym_mapping[dd->sym_count];
> > -             if (sscanf(buff, "%p %*c %s", &address, sym->name) != 2)
> > +
> > +             /* module is optional */
> > +             sym->module[0] = '\0';
> > +             /* trim the square brackets around the module name */
> > +             if (sscanf(buff, "%p %*c %s [%[^]]s", &address, sym->name, sym->module) < 2)
>
> Looking again at this patch, we should be good for parsing the module
> name with the sscanf() because I don't expect a module name longer than
> MODULE_MAX_NAME to show up, but I wonder what guarantee we have about
> symbols names staying under SYM_MAX_NAME? Maybe we should specify the
> max length to read, to remain on the safe side (or in case these limits
> change in the future). But it doesn't have to be part of your set, I can
> send a follow-up after that.

Great, thanks for your work!

-- 
Regards
Yafang

Re: [PATCH v6 bpf-next 03/11] bpftool: Show kprobe_multi link info

[Daniel Borkmann]

On 6/28/23 1:53 PM, Yafang Shao wrote:
> Show the already expose kprobe_multi link info in bpftool. The result as
> follows,
> 
> $ tools/bpf/bpftool/bpftool link show
> 91: kprobe_multi  prog 244
>          kprobe.multi  func_cnt 7
>          addr             func [module]
>          ffffffff98c44f20 schedule_timeout_interruptible
>          ffffffff98c44f60 schedule_timeout_killable
>          ffffffff98c44fa0 schedule_timeout_uninterruptible
>          ffffffff98c44fe0 schedule_timeout_idle
>          ffffffffc075b8d0 xfs_trans_get_efd [xfs]
>          ffffffffc0768a10 xfs_trans_get_buf_map [xfs]
>          ffffffffc076c320 xfs_trans_get_dqtrx [xfs]
>          pids kprobe_multi(188367)
> 92: kprobe_multi  prog 244
>          kretprobe.multi  func_cnt 7
>          addr             func [module]
>          ffffffff98c44f20 schedule_timeout_interruptible
>          ffffffff98c44f60 schedule_timeout_killable
>          ffffffff98c44fa0 schedule_timeout_uninterruptible
>          ffffffff98c44fe0 schedule_timeout_idle
>          ffffffffc075b8d0 xfs_trans_get_efd [xfs]
>          ffffffffc0768a10 xfs_trans_get_buf_map [xfs]
>          ffffffffc076c320 xfs_trans_get_dqtrx [xfs]
>          pids kprobe_multi(188367)
> 
> $ tools/bpf/bpftool/bpftool link show -j
> [{"id":91,"type":"kprobe_multi","prog_id":244,"retprobe":false,"func_cnt":7,"funcs":[{"addr":18446744071977586464,"func":"schedule_timeout_interruptible","module":null},{"addr":18446744071977586528,"func":"schedule_timeout_killable","module":null},{"addr":18446744071977586592,"func":"schedule_timeout_uninterruptible","module":null},{"addr":18446744071977586656,"func":"schedule_timeout_idle","module":null},{"addr":18446744072643524816,"func":"xfs_trans_get_efd","module":"xfs"},{"addr":18446744072643578384,"func":"xfs_trans_get_buf_map","module":"xfs"},{"addr":18446744072643592992,"func":"xfs_trans_get_dqtrx","module":"xfs"}],"pids":[{"pid":188367,"comm":"kprobe_multi"}]},{"id":92,"type":"kprobe_multi","prog_id":244,"retprobe":true,"func_cnt":7,"funcs":[{"addr":18446744071977586464,"func":"schedule_timeout_interruptible","module":null},{"addr":18446744071977586528,"func":"schedule_timeout_killable","module":null},{"addr":18446744071977586592,"func":"schedule_timeout_uninterruptible","module":null},{"addr":18446744071977586656,"func":"schedule_timeout_idle","module":null},{"addr":18446744072643524816,"func":"xfs_trans_get_efd","module":"xfs"},{"addr":18446744072643578384,"func":"xfs_trans_get_buf_map","module":"xfs"},{"addr":18446744072643592992,"func":"xfs_trans_get_dqtrx","module":"xfs"}],"pids":[{"pid":188367,"comm":"kprobe_multi"}]}]
> 
> When kptr_restrict is 2, the result is,
> 
> $ tools/bpf/bpftool/bpftool link show
> 91: kprobe_multi  prog 244
>          kprobe.multi  func_cnt 7
> 92: kprobe_multi  prog 244
>          kretprobe.multi  func_cnt 7
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Reviewed-by: Quentin Monnet <quentin@isovalent.com>

Mainly small nits, but otherwise series lgtm, thanks for improving the visibility!

>   tools/bpf/bpftool/link.c | 114 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 113 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index 2d786072ed0d..34fa17cd9d2a 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
> @@ -14,8 +14,10 @@
>   
>   #include "json_writer.h"
>   #include "main.h"
> +#include "xlated_dumper.h"
>   
>   static struct hashmap *link_table;
> +static struct dump_data dd = {};

Doesn't need explicit = {}

>   static int link_parse_fd(int *argc, char ***argv)
>   {
> @@ -166,6 +168,50 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
>   	return err;
>   }
>   
> +static int cmp_u64(const void *A, const void *B)
> +{
> +	const __u64 *a = A, *b = B;
> +
> +	return *a - *b;
> +}
> +
> +static void
> +show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
> +{
> +	__u32 i, j = 0;
> +	__u64 *addrs;
> +
> +	jsonw_bool_field(json_wtr, "retprobe",
> +			 info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
> +	jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
> +	jsonw_name(json_wtr, "funcs");
> +	jsonw_start_array(json_wtr);
> +	addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);

No need to explicitly cast.

> +	qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);

Ditto wrt cast, also sizeof(addrs[0])

> +	/* Load it once for all. */
> +	if (!dd.sym_count)
> +		kernel_syms_load(&dd);
> +	for (i = 0; i < dd.sym_count; i++) {
> +		if (dd.sym_mapping[i].address != addrs[j])
> +			continue;
> +		jsonw_start_object(json_wtr);
> +		jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
> +		jsonw_string_field(json_wtr, "func", dd.sym_mapping[i].name);
> +		/* Print null if it is vmlinux */
> +		if (dd.sym_mapping[i].module[0] == '\0') {
> +			jsonw_name(json_wtr, "module");
> +			jsonw_null(json_wtr);
> +		} else {
> +			jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
> +		}
> +		jsonw_end_object(json_wtr);
> +		if (j++ == info->kprobe_multi.count)
> +			break;
> +	}
> +	jsonw_end_array(json_wtr);
> +}
> +
>   static int show_link_close_json(int fd, struct bpf_link_info *info)
>   {
>   	struct bpf_prog_info prog_info;
> @@ -218,6 +264,9 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
>   		jsonw_uint_field(json_wtr, "map_id",
>   				 info->struct_ops.map_id);
>   		break;
> +	case BPF_LINK_TYPE_KPROBE_MULTI:
> +		show_kprobe_multi_json(info, json_wtr);
> +		break;
>   	default:
>   		break;
>   	}
> @@ -351,6 +400,44 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
>   		printf(" flags 0x%x", info->netfilter.flags);
>   }
>   
> +static void show_kprobe_multi_plain(struct bpf_link_info *info)
> +{
> +	__u32 i, j = 0;
> +	__u64 *addrs;
> +
> +	if (!info->kprobe_multi.count)
> +		return;
> +
> +	if (info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN)
> +		printf("\n\tkretprobe.multi  ");
> +	else
> +		printf("\n\tkprobe.multi  ");
> +	printf("func_cnt %u  ", info->kprobe_multi.count);
> +	addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> +	qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);

Same.

> +	/* Load it once for all. */
> +	if (!dd.sym_count)
> +		kernel_syms_load(&dd);
> +	if (!dd.sym_count)
> +		return;
> +
> +	printf("\n\t%-16s %s", "addr", "func [module]");
> +	for (i = 0; i < dd.sym_count; i++) {
> +		if (dd.sym_mapping[i].address != addrs[j])
> +			continue;
> +		printf("\n\t%016lx %s",
> +		       dd.sym_mapping[i].address, dd.sym_mapping[i].name);
> +		if (dd.sym_mapping[i].module[0] != '\0')
> +			printf(" [%s]  ", dd.sym_mapping[i].module);
> +		else
> +			printf("  ");
> +
> +		if (j++ == info->kprobe_multi.count)
> +			break;
> +	}
> +}
> +
>   static int show_link_close_plain(int fd, struct bpf_link_info *info)
>   {
>   	struct bpf_prog_info prog_info;
> @@ -396,6 +483,9 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
>   	case BPF_LINK_TYPE_NETFILTER:
>   		netfilter_dump_plain(info);
>   		break;
> +	case BPF_LINK_TYPE_KPROBE_MULTI:
> +		show_kprobe_multi_plain(info);
> +		break;
>   	default:
>   		break;
>   	}
> @@ -417,7 +507,9 @@ static int do_show_link(int fd)
>   {
>   	struct bpf_link_info info;
>   	__u32 len = sizeof(info);
> +	__u64 *addrs = NULL;
>   	char buf[256];
> +	int count;
>   	int err;
>   
>   	memset(&info, 0, sizeof(info));
> @@ -441,12 +533,28 @@ static int do_show_link(int fd)
>   		info.iter.target_name_len = sizeof(buf);
>   		goto again;
>   	}
> +	if (info.type == BPF_LINK_TYPE_KPROBE_MULTI &&
> +	    !info.kprobe_multi.addrs) {
> +		count = info.kprobe_multi.count;
> +		if (count) {
> +			addrs = calloc(count, sizeof(__u64));
> +			if (!addrs) {
> +				p_err("mem alloc failed");
> +				close(fd);
> +				return -1;

-ENOMEM

> +			}
> +			info.kprobe_multi.addrs = (unsigned long)addrs;

ptr_to_u64()


(the same should also have been used for above (unsigned long)&buf)

> +			goto again;
> +		}
> +	}
>   
>   	if (json_output)
>   		show_link_close_json(fd, &info);
>   	else
>   		show_link_close_plain(fd, &info);
>   
> +	if (addrs)
> +		free(addrs);
>   	close(fd);
>   	return 0;
>   }
> @@ -471,7 +579,8 @@ static int do_show(int argc, char **argv)
>   		fd = link_parse_fd(&argc, &argv);
>   		if (fd < 0)
>   			return fd;
> -		return do_show_link(fd);
> +		do_show_link(fd);
> +		goto out;
>   	}
>   
>   	if (argc)
> @@ -510,6 +619,9 @@ static int do_show(int argc, char **argv)
>   	if (show_pinned)
>   		delete_pinned_obj_table(link_table);
>   
> +out:
> +	if (dd.sym_count)
> +		kernel_syms_destroy(&dd);
>   	return errno == ENOENT ? 0 : -1;
>   }
>   
> 

Re: [PATCH v6 bpf-next 05/11] bpf: Clear the probe_addr for uprobe

[Daniel Borkmann]

On 6/28/23 1:53 PM, Yafang Shao wrote:
> To avoid returning uninitialized or random values when querying the file
> descriptor (fd) and accessing probe_addr, it is necessary to clear the
> variable prior to its use.
> 
> Fixes: 41bdc4b40ed6 ("bpf: introduce bpf subcommand BPF_TASK_FD_QUERY")
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Acked-by: Yonghong Song <yhs@fb.com>
> ---
>   kernel/trace/bpf_trace.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 1f9f78e1992f..ac9958907a7c 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2382,10 +2382,12 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
>   						  event->attr.type == PERF_TYPE_TRACEPOINT);
>   #endif
>   #ifdef CONFIG_UPROBE_EVENTS
> -		if (flags & TRACE_EVENT_FL_UPROBE)
> +		if (flags & TRACE_EVENT_FL_UPROBE) {
>   			err = bpf_get_uprobe_info(event, fd_type, buf,
>   						  probe_offset,
>   						  event->attr.type == PERF_TYPE_TRACEPOINT);
> +			*probe_addr = 0x0;
> +		}

Could we make this a bit more robust by just moving the zero'ing into the common path?

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 03b7f6b8e4f0..795e16d5d2f7 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2362,6 +2362,9 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
                 return -EOPNOTSUPP;

         *prog_id = prog->aux->id;
+       *probe_offset = 0x0;
+       *probe_addr = 0x0;
+
         flags = event->tp_event->flags;
         is_tracepoint = flags & TRACE_EVENT_FL_TRACEPOINT;
         is_syscall_tp = is_syscall_trace_event(event->tp_event);
@@ -2370,8 +2373,6 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
                 *buf = is_tracepoint ? event->tp_event->tp->name
                                      : event->tp_event->name;
                 *fd_type = BPF_FD_TYPE_TRACEPOINT;
-               *probe_offset = 0x0;
-               *probe_addr = 0x0;
         } else {
                 /* kprobe/uprobe */
                 err = -EOPNOTSUPP;

Re: [PATCH v6 bpf-next 06/11] bpf: Expose symbol's respective address

[Daniel Borkmann]

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 */

Re: [PATCH v6 bpf-next 09/11] bpf: Support ->fill_link_info for perf_event

[Daniel Borkmann]

On 6/28/23 1:53 PM, Yafang Shao wrote:
> By introducing support for ->fill_link_info to the perf_event link, users
> gain the ability to inspect it using `bpftool link show`. While the current
> approach involves accessing this information via `bpftool perf show`,
> consolidating link information for all link types in one place offers
> greater convenience. Additionally, this patch extends support to the
> generic perf event, which is not currently accommodated by
> `bpftool perf show`. While only the perf type and config are exposed to
> userspace, other attributes such as sample_period and sample_freq are
> ignored. It's important to note that if kptr_restrict is not permitted, the
> probed address will not be exposed, maintaining security measures.
> 
> A new enum bpf_perf_event_type is introduced to help the user understand
> which struct is relevant.
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>   include/uapi/linux/bpf.h       |  35 ++++++++++
>   kernel/bpf/syscall.c           | 117 +++++++++++++++++++++++++++++++++
>   tools/include/uapi/linux/bpf.h |  35 ++++++++++
>   3 files changed, 187 insertions(+)

For ease of review this should be squashed with the prior one which adds
bpf_perf_link_fill_common().

> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 512ba3ba2ed3..7efe51672c15 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1057,6 +1057,16 @@ enum bpf_link_type {
>   	MAX_BPF_LINK_TYPE,
>   };
>   
> +enum bpf_perf_event_type {
> +	BPF_PERF_EVENT_UNSPEC = 0,
> +	BPF_PERF_EVENT_UPROBE = 1,
> +	BPF_PERF_EVENT_URETPROBE = 2,
> +	BPF_PERF_EVENT_KPROBE = 3,
> +	BPF_PERF_EVENT_KRETPROBE = 4,
> +	BPF_PERF_EVENT_TRACEPOINT = 5,
> +	BPF_PERF_EVENT_EVENT = 6,

Why explicitly defining the values of the enum?

> +};
> +
>   /* cgroup-bpf attach flags used in BPF_PROG_ATTACH command
>    *
>    * NONE(default): No further bpf programs allowed in the subtree.
> @@ -6444,6 +6454,31 @@ struct bpf_link_info {
>   			__u32 count;
>   			__u32 flags;
>   		} kprobe_multi;
> +		struct {
> +			__u32 type; /* enum bpf_perf_event_type */
> +			__u32 :32;
> +			union {
> +				struct {
> +					__aligned_u64 file_name; /* in/out */
> +					__u32 name_len;
> +					__u32 offset;/* offset from file_name */

nit: spacing wrt comment, also same further below

> +				} uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
> +				struct {
> +					__aligned_u64 func_name; /* in/out */
> +					__u32 name_len;
> +					__u32 offset;/* offset from func_name */
> +					__u64 addr;
> +				} kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> +				struct {
> +					__aligned_u64 tp_name;   /* in/out */
> +					__u32 name_len;
> +				} tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
> +				struct {
> +					__u64 config;
> +					__u32 type;
> +				} event; /* BPF_PERF_EVENT_EVENT */
> +			};
> +		} perf_event;
>   	};
>   } __attribute__((aligned(8)));
>   
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 72de91beabbc..05ff0a560f1a 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3398,9 +3398,126 @@ static int bpf_perf_link_fill_common(const struct perf_event *event,
>   	return 0;
>   }
>   
> +#ifdef CONFIG_KPROBE_EVENTS
> +static int bpf_perf_link_fill_kprobe(const struct perf_event *event,
> +				     struct bpf_link_info *info)
> +{
> +	char __user *uname;
> +	u64 addr, offset;
> +	u32 ulen, type;
> +	int err;
> +
> +	uname = u64_to_user_ptr(info->perf_event.kprobe.func_name);
> +	ulen = info->perf_event.kprobe.name_len;
> +	err = bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
> +					&type);
> +	if (err)
> +		return err;
> +	if (type == BPF_FD_TYPE_KRETPROBE)
> +		info->perf_event.type = BPF_PERF_EVENT_KRETPROBE;
> +	else
> +		info->perf_event.type = BPF_PERF_EVENT_KPROBE;
> +
> +	info->perf_event.kprobe.offset = offset;
> +	if (!kallsyms_show_value(current_cred()))
> +		addr = 0;
> +	info->perf_event.kprobe.addr = addr;
> +	return 0;
> +}
> +#endif
> +
> +#ifdef CONFIG_UPROBE_EVENTS
> +static int bpf_perf_link_fill_uprobe(const struct perf_event *event,
> +				     struct bpf_link_info *info)
> +{
> +	char __user *uname;
> +	u64 addr, offset;
> +	u32 ulen, type;
> +	int err;
> +
> +	uname = u64_to_user_ptr(info->perf_event.uprobe.file_name);
> +	ulen = info->perf_event.uprobe.name_len;
> +	err = bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
> +					&type);
> +	if (err)
> +		return err;
> +
> +	if (type == BPF_FD_TYPE_URETPROBE)
> +		info->perf_event.type = BPF_PERF_EVENT_URETPROBE;
> +	else
> +		info->perf_event.type = BPF_PERF_EVENT_UPROBE;
> +	info->perf_event.uprobe.offset = offset;
> +	return 0;
> +}
> +#endif
> +
> +static int bpf_perf_link_fill_probe(const struct perf_event *event,
> +				    struct bpf_link_info *info)
> +{
> +#ifdef CONFIG_KPROBE_EVENTS
> +	if (event->tp_event->flags & TRACE_EVENT_FL_KPROBE)
> +		return bpf_perf_link_fill_kprobe(event, info);
> +#endif
> +#ifdef CONFIG_UPROBE_EVENTS
> +	if (event->tp_event->flags & TRACE_EVENT_FL_UPROBE)
> +		return bpf_perf_link_fill_uprobe(event, info);
> +#endif
> +	return -EOPNOTSUPP;
> +}
> +
> +static int bpf_perf_link_fill_tracepoint(const struct perf_event *event,
> +					 struct bpf_link_info *info)
> +{
> +	char __user *uname;
> +	u64 addr, offset;
> +	u32 ulen, type;
> +
> +	uname = u64_to_user_ptr(info->perf_event.tracepoint.tp_name);
> +	ulen = info->perf_event.tracepoint.name_len;
> +	info->perf_event.type = BPF_PERF_EVENT_TRACEPOINT;
> +	return bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
> +					 &type);

Perhaps for data we don't care about in these cases, passing NULL would be
more obvious and letting bpf_perf_link_fill_common() handle NULL inputs.

> +}
> +
> +static int bpf_perf_link_fill_perf_event(const struct perf_event *event,
> +					 struct bpf_link_info *info)
> +{
> +	info->perf_event.event.type = event->attr.type;
> +	info->perf_event.event.config = event->attr.config;
> +	info->perf_event.type = BPF_PERF_EVENT_EVENT;
> +	return 0;
> +}
> +
> +static int bpf_perf_link_fill_link_info(const struct bpf_link *link,
> +					struct bpf_link_info *info)
> +{
> +	struct bpf_perf_link *perf_link;
> +	const struct perf_event *event;
> +
> +	perf_link = container_of(link, struct bpf_perf_link, link);
> +	event = perf_get_event(perf_link->perf_file);
> +	if (IS_ERR(event))
> +		return PTR_ERR(event);
> +
> +	if (!event->prog)
> +		return -EINVAL;

nit: In which situations do we run into this, would ENOENT be better error code
here given it's not an invalid arg that user passed to kernel for filling link
info.

> +	switch (event->prog->type) {
> +	case BPF_PROG_TYPE_PERF_EVENT:
> +		return bpf_perf_link_fill_perf_event(event, info);
> +	case BPF_PROG_TYPE_TRACEPOINT:
> +		return bpf_perf_link_fill_tracepoint(event, info);
> +	case BPF_PROG_TYPE_KPROBE:
> +		return bpf_perf_link_fill_probe(event, info);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
>   static const struct bpf_link_ops bpf_perf_link_lops = {
>   	.release = bpf_perf_link_release,
>   	.dealloc = bpf_perf_link_dealloc,
> +	.fill_link_info = bpf_perf_link_fill_link_info,
>   };
>   
>   static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 512ba3ba2ed3..7efe51672c15 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1057,6 +1057,16 @@ enum bpf_link_type {
>   	MAX_BPF_LINK_TYPE,
>   };
>   
> +enum bpf_perf_event_type {
> +	BPF_PERF_EVENT_UNSPEC = 0,
> +	BPF_PERF_EVENT_UPROBE = 1,
> +	BPF_PERF_EVENT_URETPROBE = 2,
> +	BPF_PERF_EVENT_KPROBE = 3,
> +	BPF_PERF_EVENT_KRETPROBE = 4,
> +	BPF_PERF_EVENT_TRACEPOINT = 5,
> +	BPF_PERF_EVENT_EVENT = 6,
> +};
> +
>   /* cgroup-bpf attach flags used in BPF_PROG_ATTACH command
>    *
>    * NONE(default): No further bpf programs allowed in the subtree.
> @@ -6444,6 +6454,31 @@ struct bpf_link_info {
>   			__u32 count;
>   			__u32 flags;
>   		} kprobe_multi;
> +		struct {
> +			__u32 type; /* enum bpf_perf_event_type */
> +			__u32 :32;
> +			union {
> +				struct {
> +					__aligned_u64 file_name; /* in/out */
> +					__u32 name_len;
> +					__u32 offset;/* offset from file_name */
> +				} uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
> +				struct {
> +					__aligned_u64 func_name; /* in/out */
> +					__u32 name_len;
> +					__u32 offset;/* offset from func_name */
> +					__u64 addr;
> +				} kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> +				struct {
> +					__aligned_u64 tp_name;   /* in/out */
> +					__u32 name_len;
> +				} tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
> +				struct {
> +					__u64 config;
> +					__u32 type;
> +				} event; /* BPF_PERF_EVENT_EVENT */
> +			};
> +		} perf_event;
>   	};
>   } __attribute__((aligned(8)));
>   
> 

Re: [PATCH v6 bpf-next 11/11] bpftool: Show perf link info

[Daniel Borkmann]

On 6/28/23 1:53 PM, Yafang Shao wrote:
[...]
> +
> +static void
> +show_perf_event_event_json(struct bpf_link_info *info, json_writer_t *wtr)
> +{
> +	__u64 config = info->perf_event.event.config;
> +	__u32 type = info->perf_event.event.type;
> +	const char *perf_type, *perf_config;
> +
> +	perf_type = perf_event_name(perf_type_name, type);
> +	if (perf_type)
> +		jsonw_string_field(wtr, "event_type", perf_type);
> +	else
> +		jsonw_uint_field(wtr, "event_type", type);
> +
> +	perf_config = perf_config_str(type, config);
> +	if (perf_config)
> +		jsonw_string_field(wtr, "event_config", perf_config);
> +	else
> +		jsonw_uint_field(wtr, "event_config", config);
> +
> +	if (type == PERF_TYPE_HW_CACHE && perf_config)
> +		free((void *)perf_config);

nit no need to cast

> +}
> +
>   static int show_link_close_json(int fd, struct bpf_link_info *info)
>   {
>   	struct bpf_prog_info prog_info;
> @@ -334,6 +440,26 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
>   	case BPF_LINK_TYPE_KPROBE_MULTI:
>   		show_kprobe_multi_json(info, json_wtr);
>   		break;
> +	case BPF_LINK_TYPE_PERF_EVENT:
> +		switch (info->perf_event.type) {
> +		case BPF_PERF_EVENT_EVENT:
> +			show_perf_event_event_json(info, json_wtr);
> +			break;
> +		case BPF_PERF_EVENT_TRACEPOINT:
> +			show_perf_event_tracepoint_json(info, json_wtr);
> +			break;
> +		case BPF_PERF_EVENT_KPROBE:
> +		case BPF_PERF_EVENT_KRETPROBE:
> +			show_perf_event_kprobe_json(info, json_wtr);
> +			break;
> +		case BPF_PERF_EVENT_UPROBE:
> +		case BPF_PERF_EVENT_URETPROBE:
> +			show_perf_event_uprobe_json(info, json_wtr);
> +			break;
> +		default:
> +			break;
> +		}
> +		break;
>   	default:
>   		break;
>   	}
> @@ -505,6 +631,75 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
>   	}
>   }
>   
> +static void show_perf_event_kprobe_plain(struct bpf_link_info *info)
> +{
> +	const char *buf;
> +
> +	buf = (const char *)u64_to_ptr(info->perf_event.kprobe.func_name);

ditto, same for the other occurrences further below

> +	if (buf[0] == '\0' && !info->perf_event.kprobe.addr)
> +		return;
> +
> +	if (info->perf_event.type == BPF_PERF_EVENT_KRETPROBE)
> +		printf("\n\tkretprobe ");
> +	else
> +		printf("\n\tkprobe ");
> +	if (info->perf_event.kprobe.addr)
> +		printf("%llx ", info->perf_event.kprobe.addr);
> +	printf("%s", buf);
> +	if (info->perf_event.kprobe.offset)
> +		printf("+%#x", info->perf_event.kprobe.offset);
> +	printf("  ");
> +}
> +
> +static void show_perf_event_uprobe_plain(struct bpf_link_info *info)
> +{
> +	const char *buf;
> +
> +	buf = (const char *)u64_to_ptr(info->perf_event.uprobe.file_name);
> +	if (buf[0] == '\0')
> +		return;
> +
> +	if (info->perf_event.type == BPF_PERF_EVENT_URETPROBE)
> +		printf("\n\turetprobe ");
> +	else
> +		printf("\n\tuprobe ");
> +	printf("%s+%#x  ", buf, info->perf_event.uprobe.offset);
> +}
> +
> +static void show_perf_event_tracepoint_plain(struct bpf_link_info *info)
> +{
> +	const char *buf;
> +
> +	buf = (const char *)u64_to_ptr(info->perf_event.tracepoint.tp_name);
> +	if (buf[0] == '\0')
> +		return;
> +
> +	printf("\n\ttracepoint %s  ", buf);
> +}
> +
> +static void show_perf_event_event_plain(struct bpf_link_info *info)
> +{
> +	__u64 config = info->perf_event.event.config;
> +	__u32 type = info->perf_event.event.type;
> +	const char *perf_type, *perf_config;
> +
> +	printf("\n\tevent ");
> +	perf_type = perf_event_name(perf_type_name, type);
> +	if (perf_type)
> +		printf("%s:", perf_type);
> +	else
> +		printf("%u :", type);
> +
> +	perf_config = perf_config_str(type, config);
> +	if (perf_config)
> +		printf("%s  ", perf_config);
> +	else
> +		printf("%llu  ", config);
> +
> +	if (type == PERF_TYPE_HW_CACHE && perf_config)
> +		free((void *)perf_config);

same

> +}
> +
>   static int show_link_close_plain(int fd, struct bpf_link_info *info)
>   {
>   	struct bpf_prog_info prog_info;
> @@ -553,6 +748,26 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
>   	case BPF_LINK_TYPE_KPROBE_MULTI:
>   		show_kprobe_multi_plain(info);
>   		break;
> +	case BPF_LINK_TYPE_PERF_EVENT:
> +		switch (info->perf_event.type) {
> +		case BPF_PERF_EVENT_EVENT:
> +			show_perf_event_event_plain(info);
> +			break;
> +		case BPF_PERF_EVENT_TRACEPOINT:
> +			show_perf_event_tracepoint_plain(info);
> +			break;
> +		case BPF_PERF_EVENT_KPROBE:
> +		case BPF_PERF_EVENT_KRETPROBE:
> +			show_perf_event_kprobe_plain(info);
> +			break;
> +		case BPF_PERF_EVENT_UPROBE:
> +		case BPF_PERF_EVENT_URETPROBE:
> +			show_perf_event_uprobe_plain(info);
> +			break;
> +		default:
> +			break;
> +		}
> +		break;
>   	default:
>   		break;
>   	}
> @@ -575,11 +790,12 @@ static int do_show_link(int fd)
>   	struct bpf_link_info info;
>   	__u32 len = sizeof(info);
>   	__u64 *addrs = NULL;
> -	char buf[256];
> +	char buf[PATH_MAX];
>   	int count;
>   	int err;
>   
>   	memset(&info, 0, sizeof(info));
> +	buf[0] = '\0';
>   again:
>   	err = bpf_link_get_info_by_fd(fd, &info, &len);
>   	if (err) {
> @@ -614,6 +830,35 @@ static int do_show_link(int fd)
>   			goto again;
>   		}
>   	}
> +	if (info.type == BPF_LINK_TYPE_PERF_EVENT) {
> +		switch (info.perf_event.type) {
> +		case BPF_PERF_EVENT_TRACEPOINT:
> +			if (!info.perf_event.tracepoint.tp_name) {
> +				info.perf_event.tracepoint.tp_name = (unsigned long)&buf;

lets use ptr_to_u64() in all these cases

> +				info.perf_event.tracepoint.name_len = sizeof(buf);
> +				goto again;
> +			}
> +			break;
> +		case BPF_PERF_EVENT_KPROBE:
> +		case BPF_PERF_EVENT_KRETPROBE:
> +			if (!info.perf_event.kprobe.func_name) {
> +				info.perf_event.kprobe.func_name = (unsigned long)&buf;
> +				info.perf_event.kprobe.name_len = sizeof(buf);
> +				goto again;
> +			}
> +			break;
> +		case BPF_PERF_EVENT_UPROBE:
> +		case BPF_PERF_EVENT_URETPROBE:
> +			if (!info.perf_event.uprobe.file_name) {
> +				info.perf_event.uprobe.file_name = (unsigned long)&buf;
> +				info.perf_event.uprobe.name_len = sizeof(buf);
> +				goto again;
> +			}
> +			break;
> +		default:
> +			break;
> +		}
> +	}
>   
>   	if (json_output)
>   		show_link_close_json(fd, &info);
> 

Re: [PATCH v6 bpf-next 03/11] bpftool: Show kprobe_multi link info

[Yafang Shao]

On Wed, Jul 5, 2023 at 4:09 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 6/28/23 1:53 PM, Yafang Shao wrote:
> > Show the already expose kprobe_multi link info in bpftool. The result as
> > follows,
> >
> > $ tools/bpf/bpftool/bpftool link show
> > 91: kprobe_multi  prog 244
> >          kprobe.multi  func_cnt 7
> >          addr             func [module]
> >          ffffffff98c44f20 schedule_timeout_interruptible
> >          ffffffff98c44f60 schedule_timeout_killable
> >          ffffffff98c44fa0 schedule_timeout_uninterruptible
> >          ffffffff98c44fe0 schedule_timeout_idle
> >          ffffffffc075b8d0 xfs_trans_get_efd [xfs]
> >          ffffffffc0768a10 xfs_trans_get_buf_map [xfs]
> >          ffffffffc076c320 xfs_trans_get_dqtrx [xfs]
> >          pids kprobe_multi(188367)
> > 92: kprobe_multi  prog 244
> >          kretprobe.multi  func_cnt 7
> >          addr             func [module]
> >          ffffffff98c44f20 schedule_timeout_interruptible
> >          ffffffff98c44f60 schedule_timeout_killable
> >          ffffffff98c44fa0 schedule_timeout_uninterruptible
> >          ffffffff98c44fe0 schedule_timeout_idle
> >          ffffffffc075b8d0 xfs_trans_get_efd [xfs]
> >          ffffffffc0768a10 xfs_trans_get_buf_map [xfs]
> >          ffffffffc076c320 xfs_trans_get_dqtrx [xfs]
> >          pids kprobe_multi(188367)
> >
> > $ tools/bpf/bpftool/bpftool link show -j
> > [{"id":91,"type":"kprobe_multi","prog_id":244,"retprobe":false,"func_cnt":7,"funcs":[{"addr":18446744071977586464,"func":"schedule_timeout_interruptible","module":null},{"addr":18446744071977586528,"func":"schedule_timeout_killable","module":null},{"addr":18446744071977586592,"func":"schedule_timeout_uninterruptible","module":null},{"addr":18446744071977586656,"func":"schedule_timeout_idle","module":null},{"addr":18446744072643524816,"func":"xfs_trans_get_efd","module":"xfs"},{"addr":18446744072643578384,"func":"xfs_trans_get_buf_map","module":"xfs"},{"addr":18446744072643592992,"func":"xfs_trans_get_dqtrx","module":"xfs"}],"pids":[{"pid":188367,"comm":"kprobe_multi"}]},{"id":92,"type":"kprobe_multi","prog_id":244,"retprobe":true,"func_cnt":7,"funcs":[{"addr":18446744071977586464,"func":"schedule_timeout_interruptible","module":null},{"addr":18446744071977586528,"func":"schedule_timeout_killable","module":null},{"addr":18446744071977586592,"func":"schedule_timeout_uninterruptible","module":null},{"addr":18446744071977586656,"func":"schedule_timeout_idle","module":null},{"addr":18446744072643524816,"func":"xfs_trans_get_efd","module":"xfs"},{"addr":18446744072643578384,"func":"xfs_trans_get_buf_map","module":"xfs"},{"addr":18446744072643592992,"func":"xfs_trans_get_dqtrx","module":"xfs"}],"pids":[{"pid":188367,"comm":"kprobe_multi"}]}]
> >
> > When kptr_restrict is 2, the result is,
> >
> > $ tools/bpf/bpftool/bpftool link show
> > 91: kprobe_multi  prog 244
> >          kprobe.multi  func_cnt 7
> > 92: kprobe_multi  prog 244
> >          kretprobe.multi  func_cnt 7
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Reviewed-by: Quentin Monnet <quentin@isovalent.com>
>
> Mainly small nits, but otherwise series lgtm, thanks for improving the visibility!

Will change them in the next version. Thanks for your review.

-- 
Regards
Yafang

Re: [PATCH v6 bpf-next 05/11] bpf: Clear the probe_addr for uprobe

[Yafang Shao]

On Wed, Jul 5, 2023 at 4:19 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 6/28/23 1:53 PM, Yafang Shao wrote:
> > To avoid returning uninitialized or random values when querying the file
> > descriptor (fd) and accessing probe_addr, it is necessary to clear the
> > variable prior to its use.
> >
> > Fixes: 41bdc4b40ed6 ("bpf: introduce bpf subcommand BPF_TASK_FD_QUERY")
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Acked-by: Yonghong Song <yhs@fb.com>
> > ---
> >   kernel/trace/bpf_trace.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 1f9f78e1992f..ac9958907a7c 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2382,10 +2382,12 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
> >                                                 event->attr.type == PERF_TYPE_TRACEPOINT);
> >   #endif
> >   #ifdef CONFIG_UPROBE_EVENTS
> > -             if (flags & TRACE_EVENT_FL_UPROBE)
> > +             if (flags & TRACE_EVENT_FL_UPROBE) {
> >                       err = bpf_get_uprobe_info(event, fd_type, buf,
> >                                                 probe_offset,
> >                                                 event->attr.type == PERF_TYPE_TRACEPOINT);
> > +                     *probe_addr = 0x0;
> > +             }
>
> Could we make this a bit more robust by just moving the zero'ing into the common path?

Agree. Will change it.

>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 03b7f6b8e4f0..795e16d5d2f7 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2362,6 +2362,9 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
>                  return -EOPNOTSUPP;
>
>          *prog_id = prog->aux->id;
> +       *probe_offset = 0x0;
> +       *probe_addr = 0x0;
> +
>          flags = event->tp_event->flags;
>          is_tracepoint = flags & TRACE_EVENT_FL_TRACEPOINT;
>          is_syscall_tp = is_syscall_trace_event(event->tp_event);
> @@ -2370,8 +2373,6 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
>                  *buf = is_tracepoint ? event->tp_event->tp->name
>                                       : event->tp_event->name;
>                  *fd_type = BPF_FD_TYPE_TRACEPOINT;
> -               *probe_offset = 0x0;
> -               *probe_addr = 0x0;
>          } else {
>                  /* kprobe/uprobe */
>                  err = -EOPNOTSUPP;



-- 
Regards
Yafang

Re: [PATCH v6 bpf-next 06/11] bpf: Expose symbol's respective address

[Yafang Shao]

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 */
>


-- 
Regards
Yafang

Re: [PATCH v6 bpf-next 09/11] bpf: Support ->fill_link_info for perf_event

[Yafang Shao]

On Wed, Jul 5, 2023 at 4:47 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 6/28/23 1:53 PM, Yafang Shao wrote:
> > By introducing support for ->fill_link_info to the perf_event link, users
> > gain the ability to inspect it using `bpftool link show`. While the current
> > approach involves accessing this information via `bpftool perf show`,
> > consolidating link information for all link types in one place offers
> > greater convenience. Additionally, this patch extends support to the
> > generic perf event, which is not currently accommodated by
> > `bpftool perf show`. While only the perf type and config are exposed to
> > userspace, other attributes such as sample_period and sample_freq are
> > ignored. It's important to note that if kptr_restrict is not permitted, the
> > probed address will not be exposed, maintaining security measures.
> >
> > A new enum bpf_perf_event_type is introduced to help the user understand
> > which struct is relevant.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> >   include/uapi/linux/bpf.h       |  35 ++++++++++
> >   kernel/bpf/syscall.c           | 117 +++++++++++++++++++++++++++++++++
> >   tools/include/uapi/linux/bpf.h |  35 ++++++++++
> >   3 files changed, 187 insertions(+)
>
> For ease of review this should be squashed with the prior one which adds
> bpf_perf_link_fill_common().

Sure. Will do it.

>
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 512ba3ba2ed3..7efe51672c15 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1057,6 +1057,16 @@ enum bpf_link_type {
> >       MAX_BPF_LINK_TYPE,
> >   };
> >
> > +enum bpf_perf_event_type {
> > +     BPF_PERF_EVENT_UNSPEC = 0,
> > +     BPF_PERF_EVENT_UPROBE = 1,
> > +     BPF_PERF_EVENT_URETPROBE = 2,
> > +     BPF_PERF_EVENT_KPROBE = 3,
> > +     BPF_PERF_EVENT_KRETPROBE = 4,
> > +     BPF_PERF_EVENT_TRACEPOINT = 5,
> > +     BPF_PERF_EVENT_EVENT = 6,
>
> Why explicitly defining the values of the enum?

With these newly introduced enums, the user can easily identify what
kind of perf_event link it is
See also the discussion:
https://lore.kernel.org/bpf/CAEf4BzYEwCZ3J51pFnUfGykEAHtdLwB8Kxi0utvUTVvewz4UCg@mail.gmail.com/

>
> > +};
> > +
> >   /* cgroup-bpf attach flags used in BPF_PROG_ATTACH command
> >    *
> >    * NONE(default): No further bpf programs allowed in the subtree.
> > @@ -6444,6 +6454,31 @@ struct bpf_link_info {
> >                       __u32 count;
> >                       __u32 flags;
> >               } kprobe_multi;
> > +             struct {
> > +                     __u32 type; /* enum bpf_perf_event_type */
> > +                     __u32 :32;
> > +                     union {
> > +                             struct {
> > +                                     __aligned_u64 file_name; /* in/out */
> > +                                     __u32 name_len;
> > +                                     __u32 offset;/* offset from file_name */
>
> nit: spacing wrt comment, also same further below

Will change it.

>
> > +                             } uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
> > +                             struct {
> > +                                     __aligned_u64 func_name; /* in/out */
> > +                                     __u32 name_len;
> > +                                     __u32 offset;/* offset from func_name */
> > +                                     __u64 addr;
> > +                             } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> > +                             struct {
> > +                                     __aligned_u64 tp_name;   /* in/out */
> > +                                     __u32 name_len;
> > +                             } tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
> > +                             struct {
> > +                                     __u64 config;
> > +                                     __u32 type;
> > +                             } event; /* BPF_PERF_EVENT_EVENT */
> > +                     };
> > +             } perf_event;
> >       };
> >   } __attribute__((aligned(8)));
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 72de91beabbc..05ff0a560f1a 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3398,9 +3398,126 @@ static int bpf_perf_link_fill_common(const struct perf_event *event,
> >       return 0;
> >   }
> >
> > +#ifdef CONFIG_KPROBE_EVENTS
> > +static int bpf_perf_link_fill_kprobe(const struct perf_event *event,
> > +                                  struct bpf_link_info *info)
> > +{
> > +     char __user *uname;
> > +     u64 addr, offset;
> > +     u32 ulen, type;
> > +     int err;
> > +
> > +     uname = u64_to_user_ptr(info->perf_event.kprobe.func_name);
> > +     ulen = info->perf_event.kprobe.name_len;
> > +     err = bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
> > +                                     &type);
> > +     if (err)
> > +             return err;
> > +     if (type == BPF_FD_TYPE_KRETPROBE)
> > +             info->perf_event.type = BPF_PERF_EVENT_KRETPROBE;
> > +     else
> > +             info->perf_event.type = BPF_PERF_EVENT_KPROBE;
> > +
> > +     info->perf_event.kprobe.offset = offset;
> > +     if (!kallsyms_show_value(current_cred()))
> > +             addr = 0;
> > +     info->perf_event.kprobe.addr = addr;
> > +     return 0;
> > +}
> > +#endif
> > +
> > +#ifdef CONFIG_UPROBE_EVENTS
> > +static int bpf_perf_link_fill_uprobe(const struct perf_event *event,
> > +                                  struct bpf_link_info *info)
> > +{
> > +     char __user *uname;
> > +     u64 addr, offset;
> > +     u32 ulen, type;
> > +     int err;
> > +
> > +     uname = u64_to_user_ptr(info->perf_event.uprobe.file_name);
> > +     ulen = info->perf_event.uprobe.name_len;
> > +     err = bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
> > +                                     &type);
> > +     if (err)
> > +             return err;
> > +
> > +     if (type == BPF_FD_TYPE_URETPROBE)
> > +             info->perf_event.type = BPF_PERF_EVENT_URETPROBE;
> > +     else
> > +             info->perf_event.type = BPF_PERF_EVENT_UPROBE;
> > +     info->perf_event.uprobe.offset = offset;
> > +     return 0;
> > +}
> > +#endif
> > +
> > +static int bpf_perf_link_fill_probe(const struct perf_event *event,
> > +                                 struct bpf_link_info *info)
> > +{
> > +#ifdef CONFIG_KPROBE_EVENTS
> > +     if (event->tp_event->flags & TRACE_EVENT_FL_KPROBE)
> > +             return bpf_perf_link_fill_kprobe(event, info);
> > +#endif
> > +#ifdef CONFIG_UPROBE_EVENTS
> > +     if (event->tp_event->flags & TRACE_EVENT_FL_UPROBE)
> > +             return bpf_perf_link_fill_uprobe(event, info);
> > +#endif
> > +     return -EOPNOTSUPP;
> > +}
> > +
> > +static int bpf_perf_link_fill_tracepoint(const struct perf_event *event,
> > +                                      struct bpf_link_info *info)
> > +{
> > +     char __user *uname;
> > +     u64 addr, offset;
> > +     u32 ulen, type;
> > +
> > +     uname = u64_to_user_ptr(info->perf_event.tracepoint.tp_name);
> > +     ulen = info->perf_event.tracepoint.name_len;
> > +     info->perf_event.type = BPF_PERF_EVENT_TRACEPOINT;
> > +     return bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
> > +                                      &type);
>
> Perhaps for data we don't care about in these cases, passing NULL would be
> more obvious and letting bpf_perf_link_fill_common() handle NULL inputs.

Agree. That would be better.
We should let bpf_get_perf_event_info() handle NULL inputs.  As the
change in bpf_get_perf_event_info() is small, I will change it in the
same patch.

>
> > +}
> > +
> > +static int bpf_perf_link_fill_perf_event(const struct perf_event *event,
> > +                                      struct bpf_link_info *info)
> > +{
> > +     info->perf_event.event.type = event->attr.type;
> > +     info->perf_event.event.config = event->attr.config;
> > +     info->perf_event.type = BPF_PERF_EVENT_EVENT;
> > +     return 0;
> > +}
> > +
> > +static int bpf_perf_link_fill_link_info(const struct bpf_link *link,
> > +                                     struct bpf_link_info *info)
> > +{
> > +     struct bpf_perf_link *perf_link;
> > +     const struct perf_event *event;
> > +
> > +     perf_link = container_of(link, struct bpf_perf_link, link);
> > +     event = perf_get_event(perf_link->perf_file);
> > +     if (IS_ERR(event))
> > +             return PTR_ERR(event);
> > +
> > +     if (!event->prog)
> > +             return -EINVAL;
>
> nit: In which situations do we run into this, would ENOENT be better error code
> here given it's not an invalid arg that user passed to kernel for filling link
> info.

In practice there should be no situations. I think we can remove this
judgement directly.

-- 
Regards
Yafang

Re: [PATCH v6 bpf-next 11/11] bpftool: Show perf link info

[Yafang Shao]

On Wed, Jul 5, 2023 at 4:54 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 6/28/23 1:53 PM, Yafang Shao wrote:
> [...]
> > +
> > +static void
> > +show_perf_event_event_json(struct bpf_link_info *info, json_writer_t *wtr)
> > +{
> > +     __u64 config = info->perf_event.event.config;
> > +     __u32 type = info->perf_event.event.type;
> > +     const char *perf_type, *perf_config;
> > +
> > +     perf_type = perf_event_name(perf_type_name, type);
> > +     if (perf_type)
> > +             jsonw_string_field(wtr, "event_type", perf_type);
> > +     else
> > +             jsonw_uint_field(wtr, "event_type", type);
> > +
> > +     perf_config = perf_config_str(type, config);
> > +     if (perf_config)
> > +             jsonw_string_field(wtr, "event_config", perf_config);
> > +     else
> > +             jsonw_uint_field(wtr, "event_config", config);
> > +
> > +     if (type == PERF_TYPE_HW_CACHE && perf_config)
> > +             free((void *)perf_config);
>
> nit no need to cast

It will discard the 'const', so we have to do the cast here, otherwise
the compiler will complain.

>
> > +}
> > +
> >   static int show_link_close_json(int fd, struct bpf_link_info *info)
> >   {
> >       struct bpf_prog_info prog_info;
> > @@ -334,6 +440,26 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
> >       case BPF_LINK_TYPE_KPROBE_MULTI:
> >               show_kprobe_multi_json(info, json_wtr);
> >               break;
> > +     case BPF_LINK_TYPE_PERF_EVENT:
> > +             switch (info->perf_event.type) {
> > +             case BPF_PERF_EVENT_EVENT:
> > +                     show_perf_event_event_json(info, json_wtr);
> > +                     break;
> > +             case BPF_PERF_EVENT_TRACEPOINT:
> > +                     show_perf_event_tracepoint_json(info, json_wtr);
> > +                     break;
> > +             case BPF_PERF_EVENT_KPROBE:
> > +             case BPF_PERF_EVENT_KRETPROBE:
> > +                     show_perf_event_kprobe_json(info, json_wtr);
> > +                     break;
> > +             case BPF_PERF_EVENT_UPROBE:
> > +             case BPF_PERF_EVENT_URETPROBE:
> > +                     show_perf_event_uprobe_json(info, json_wtr);
> > +                     break;
> > +             default:
> > +                     break;
> > +             }
> > +             break;
> >       default:
> >               break;
> >       }
> > @@ -505,6 +631,75 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
> >       }
> >   }
> >
> > +static void show_perf_event_kprobe_plain(struct bpf_link_info *info)
> > +{
> > +     const char *buf;
> > +
> > +     buf = (const char *)u64_to_ptr(info->perf_event.kprobe.func_name);
>
> ditto, same for the other occurrences further below

Agree.

>
> > +     if (buf[0] == '\0' && !info->perf_event.kprobe.addr)
> > +             return;
> > +
> > +     if (info->perf_event.type == BPF_PERF_EVENT_KRETPROBE)
> > +             printf("\n\tkretprobe ");
> > +     else
> > +             printf("\n\tkprobe ");
> > +     if (info->perf_event.kprobe.addr)
> > +             printf("%llx ", info->perf_event.kprobe.addr);
> > +     printf("%s", buf);
> > +     if (info->perf_event.kprobe.offset)
> > +             printf("+%#x", info->perf_event.kprobe.offset);
> > +     printf("  ");
> > +}
> > +
> > +static void show_perf_event_uprobe_plain(struct bpf_link_info *info)
> > +{
> > +     const char *buf;
> > +
> > +     buf = (const char *)u64_to_ptr(info->perf_event.uprobe.file_name);
> > +     if (buf[0] == '\0')
> > +             return;
> > +
> > +     if (info->perf_event.type == BPF_PERF_EVENT_URETPROBE)
> > +             printf("\n\turetprobe ");
> > +     else
> > +             printf("\n\tuprobe ");
> > +     printf("%s+%#x  ", buf, info->perf_event.uprobe.offset);
> > +}
> > +
> > +static void show_perf_event_tracepoint_plain(struct bpf_link_info *info)
> > +{
> > +     const char *buf;
> > +
> > +     buf = (const char *)u64_to_ptr(info->perf_event.tracepoint.tp_name);
> > +     if (buf[0] == '\0')
> > +             return;
> > +
> > +     printf("\n\ttracepoint %s  ", buf);
> > +}
> > +
> > +static void show_perf_event_event_plain(struct bpf_link_info *info)
> > +{
> > +     __u64 config = info->perf_event.event.config;
> > +     __u32 type = info->perf_event.event.type;
> > +     const char *perf_type, *perf_config;
> > +
> > +     printf("\n\tevent ");
> > +     perf_type = perf_event_name(perf_type_name, type);
> > +     if (perf_type)
> > +             printf("%s:", perf_type);
> > +     else
> > +             printf("%u :", type);
> > +
> > +     perf_config = perf_config_str(type, config);
> > +     if (perf_config)
> > +             printf("%s  ", perf_config);
> > +     else
> > +             printf("%llu  ", config);
> > +
> > +     if (type == PERF_TYPE_HW_CACHE && perf_config)
> > +             free((void *)perf_config);
>
> same
>
> > +}
> > +
> >   static int show_link_close_plain(int fd, struct bpf_link_info *info)
> >   {
> >       struct bpf_prog_info prog_info;
> > @@ -553,6 +748,26 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
> >       case BPF_LINK_TYPE_KPROBE_MULTI:
> >               show_kprobe_multi_plain(info);
> >               break;
> > +     case BPF_LINK_TYPE_PERF_EVENT:
> > +             switch (info->perf_event.type) {
> > +             case BPF_PERF_EVENT_EVENT:
> > +                     show_perf_event_event_plain(info);
> > +                     break;
> > +             case BPF_PERF_EVENT_TRACEPOINT:
> > +                     show_perf_event_tracepoint_plain(info);
> > +                     break;
> > +             case BPF_PERF_EVENT_KPROBE:
> > +             case BPF_PERF_EVENT_KRETPROBE:
> > +                     show_perf_event_kprobe_plain(info);
> > +                     break;
> > +             case BPF_PERF_EVENT_UPROBE:
> > +             case BPF_PERF_EVENT_URETPROBE:
> > +                     show_perf_event_uprobe_plain(info);
> > +                     break;
> > +             default:
> > +                     break;
> > +             }
> > +             break;
> >       default:
> >               break;
> >       }
> > @@ -575,11 +790,12 @@ static int do_show_link(int fd)
> >       struct bpf_link_info info;
> >       __u32 len = sizeof(info);
> >       __u64 *addrs = NULL;
> > -     char buf[256];
> > +     char buf[PATH_MAX];
> >       int count;
> >       int err;
> >
> >       memset(&info, 0, sizeof(info));
> > +     buf[0] = '\0';
> >   again:
> >       err = bpf_link_get_info_by_fd(fd, &info, &len);
> >       if (err) {
> > @@ -614,6 +830,35 @@ static int do_show_link(int fd)
> >                       goto again;
> >               }
> >       }
> > +     if (info.type == BPF_LINK_TYPE_PERF_EVENT) {
> > +             switch (info.perf_event.type) {
> > +             case BPF_PERF_EVENT_TRACEPOINT:
> > +                     if (!info.perf_event.tracepoint.tp_name) {
> > +                             info.perf_event.tracepoint.tp_name = (unsigned long)&buf;
>
> lets use ptr_to_u64() in all these cases

Agree.

>
> > +                             info.perf_event.tracepoint.name_len = sizeof(buf);
> > +                             goto again;
> > +                     }
> > +                     break;
> > +             case BPF_PERF_EVENT_KPROBE:
> > +             case BPF_PERF_EVENT_KRETPROBE:
> > +                     if (!info.perf_event.kprobe.func_name) {
> > +                             info.perf_event.kprobe.func_name = (unsigned long)&buf;
> > +                             info.perf_event.kprobe.name_len = sizeof(buf);
> > +                             goto again;
> > +                     }
> > +                     break;
> > +             case BPF_PERF_EVENT_UPROBE:
> > +             case BPF_PERF_EVENT_URETPROBE:
> > +                     if (!info.perf_event.uprobe.file_name) {
> > +                             info.perf_event.uprobe.file_name = (unsigned long)&buf;
> > +                             info.perf_event.uprobe.name_len = sizeof(buf);
> > +                             goto again;
> > +                     }
> > +                     break;
> > +             default:
> > +                     break;
> > +             }
> > +     }
> >
> >       if (json_output)
> >               show_link_close_json(fd, &info);
> >
>


-- 
Regards
Yafang

Re: [PATCH v6 bpf-next 09/11] bpf: Support ->fill_link_info for perf_event

[Daniel Borkmann]

On 7/5/23 12:08 PM, Yafang Shao wrote:
> On Wed, Jul 5, 2023 at 4:47 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 6/28/23 1:53 PM, Yafang Shao wrote:
>>> By introducing support for ->fill_link_info to the perf_event link, users
>>> gain the ability to inspect it using `bpftool link show`. While the current
>>> approach involves accessing this information via `bpftool perf show`,
>>> consolidating link information for all link types in one place offers
>>> greater convenience. Additionally, this patch extends support to the
>>> generic perf event, which is not currently accommodated by
>>> `bpftool perf show`. While only the perf type and config are exposed to
>>> userspace, other attributes such as sample_period and sample_freq are
>>> ignored. It's important to note that if kptr_restrict is not permitted, the
>>> probed address will not be exposed, maintaining security measures.
>>>
>>> A new enum bpf_perf_event_type is introduced to help the user understand
>>> which struct is relevant.
>>>
>>> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
[...]
>>>
>>> +enum bpf_perf_event_type {
>>> +     BPF_PERF_EVENT_UNSPEC = 0,
>>> +     BPF_PERF_EVENT_UPROBE = 1,
>>> +     BPF_PERF_EVENT_URETPROBE = 2,
>>> +     BPF_PERF_EVENT_KPROBE = 3,
>>> +     BPF_PERF_EVENT_KRETPROBE = 4,
>>> +     BPF_PERF_EVENT_TRACEPOINT = 5,
>>> +     BPF_PERF_EVENT_EVENT = 6,
>>
>> Why explicitly defining the values of the enum?
> 
> With these newly introduced enums, the user can easily identify what
> kind of perf_event link it is
> See also the discussion:
> https://lore.kernel.org/bpf/CAEf4BzYEwCZ3J51pFnUfGykEAHtdLwB8Kxi0utvUTVvewz4UCg@mail.gmail.com/

No objections to that. I was more wondering why explicitly stating the
numbers here, but I presume it's for quick readability.. looks like in
some of the uapi enums we do it, in some others we don't; fair enough.

Thanks,
Daniel

Re: [PATCH v6 bpf-next 05/11] bpf: Clear the probe_addr for uprobe

[Yafang Shao]

On Wed, Jul 5, 2023 at 4:19 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 6/28/23 1:53 PM, Yafang Shao wrote:
> > To avoid returning uninitialized or random values when querying the file
> > descriptor (fd) and accessing probe_addr, it is necessary to clear the
> > variable prior to its use.
> >
> > Fixes: 41bdc4b40ed6 ("bpf: introduce bpf subcommand BPF_TASK_FD_QUERY")
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Acked-by: Yonghong Song <yhs@fb.com>
> > ---
> >   kernel/trace/bpf_trace.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 1f9f78e1992f..ac9958907a7c 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2382,10 +2382,12 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
> >                                                 event->attr.type == PERF_TYPE_TRACEPOINT);
> >   #endif
> >   #ifdef CONFIG_UPROBE_EVENTS
> > -             if (flags & TRACE_EVENT_FL_UPROBE)
> > +             if (flags & TRACE_EVENT_FL_UPROBE) {
> >                       err = bpf_get_uprobe_info(event, fd_type, buf,
> >                                                 probe_offset,
> >                                                 event->attr.type == PERF_TYPE_TRACEPOINT);
> > +                     *probe_addr = 0x0;
> > +             }
>
> Could we make this a bit more robust by just moving the zero'ing into the common path?

After a second thought, I prefer to clear it in bpf_get_uprobe_info().
That way we can avoid setting them twice for kprobe.

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 8b92e34..015dbf2 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -1432,6 +1432,7 @@ int bpf_get_uprobe_info(const struct perf_event
*event, u32 *fd_type,
                                    : BPF_FD_TYPE_UPROBE;
        *filename = tu->filename;
        *probe_offset = tu->offset;
+       *probe_addr = 0;
        return 0;
 }
 #endif /* CONFIG_PERF_EVENTS */


>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 03b7f6b8e4f0..795e16d5d2f7 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2362,6 +2362,9 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
>                  return -EOPNOTSUPP;
>
>          *prog_id = prog->aux->id;
> +       *probe_offset = 0x0;
> +       *probe_addr = 0x0;
> +
>          flags = event->tp_event->flags;
>          is_tracepoint = flags & TRACE_EVENT_FL_TRACEPOINT;
>          is_syscall_tp = is_syscall_trace_event(event->tp_event);
> @@ -2370,8 +2373,6 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
>                  *buf = is_tracepoint ? event->tp_event->tp->name
>                                       : event->tp_event->name;
>                  *fd_type = BPF_FD_TYPE_TRACEPOINT;
> -               *probe_offset = 0x0;
> -               *probe_addr = 0x0;
>          } else {
>                  /* kprobe/uprobe */
>                  err = -EOPNOTSUPP;



-- 
Regards
Yafang

Re: [PATCH v6 bpf-next 01/11] bpf: Support ->fill_link_info for kprobe_multi

[Andrii Nakryiko]

On Wed, Jun 28, 2023 at 4:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> With the addition of support for fill_link_info to the kprobe_multi link,
> users will gain the ability to inspect it conveniently using the
> `bpftool link show`. This enhancement provides valuable information to the
> user, including the count of probed functions and their respective
> addresses. It's important to note that if the kptr_restrict setting is not
> permitted, the probed address will not be exposed, ensuring security.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---

documentation nit, but otherwise LGTM

Also, looking at other patch where you introduce bpf_copy_user(),
seems like we return -ENOSPC when user-provided memory is not big
enough. So let's change E2BIG in this patch to ENOSPC?


Acked-by: Andrii Nakryiko <andrii@kernel.org>

>  include/uapi/linux/bpf.h       |  5 +++++
>  kernel/trace/bpf_trace.c       | 37 ++++++++++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h |  5 +++++
>  3 files changed, 47 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 60a9d59beeab..512ba3ba2ed3 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6439,6 +6439,11 @@ struct bpf_link_info {
>                         __s32 priority;
>                         __u32 flags;
>                 } netfilter;
> +               struct {
> +                       __aligned_u64 addrs; /* in/out: addresses buffer ptr */

addrs field itself is not modified, the memory it points to is
modified, so it's not really in/out per se, it is just a pointer to
memory where kernel stores output data

> +                       __u32 count;

but count field itself is in/out, so please add a comment explicitly
stating this


> +                       __u32 flags;
> +               } kprobe_multi;
>         };
>  } __attribute__((aligned(8)));
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 03b7f6b8e4f0..1f9f78e1992f 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2469,6 +2469,7 @@ struct bpf_kprobe_multi_link {
>         u32 cnt;
>         u32 mods_cnt;
>         struct module **mods;
> +       u32 flags;
>  };
>
>  struct bpf_kprobe_multi_run_ctx {
> @@ -2558,9 +2559,44 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
>         kfree(kmulti_link);
>  }
>

[...]

Re: [PATCH v6 bpf-next 07/11] bpf: Add a common helper bpf_copy_to_user()

[Andrii Nakryiko]

On Wed, Jun 28, 2023 at 4:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> Add a common helper bpf_copy_to_user(), which will be used at multiple
> places.
> No functional change.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  kernel/bpf/syscall.c | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
>

Acked-by: Andrii Nakryiko <andrii@kernel.org>



> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index a2aef900519c..4aa6e5776a04 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3295,6 +3295,25 @@ static void bpf_raw_tp_link_show_fdinfo(const struct bpf_link *link,
>                    raw_tp_link->btp->tp->name);
>  }
>
> +static int bpf_copy_to_user(char __user *ubuf, const char *buf, u32 ulen,
> +                           u32 len)
> +{
> +       if (ulen >= len + 1) {
> +               if (copy_to_user(ubuf, buf, len + 1))
> +                       return -EFAULT;
> +       } else {
> +               char zero = '\0';
> +
> +               if (copy_to_user(ubuf, buf, ulen - 1))
> +                       return -EFAULT;
> +               if (put_user(zero, ubuf + ulen - 1))
> +                       return -EFAULT;
> +               return -ENOSPC;
> +       }
> +
> +       return 0;
> +}
> +
>  static int bpf_raw_tp_link_fill_link_info(const struct bpf_link *link,
>                                           struct bpf_link_info *info)
>  {
> @@ -3313,20 +3332,7 @@ static int bpf_raw_tp_link_fill_link_info(const struct bpf_link *link,
>         if (!ubuf)
>                 return 0;
>
> -       if (ulen >= tp_len + 1) {
> -               if (copy_to_user(ubuf, tp_name, tp_len + 1))
> -                       return -EFAULT;
> -       } else {
> -               char zero = '\0';
> -
> -               if (copy_to_user(ubuf, tp_name, ulen - 1))
> -                       return -EFAULT;
> -               if (put_user(zero, ubuf + ulen - 1))
> -                       return -EFAULT;
> -               return -ENOSPC;
> -       }
> -
> -       return 0;
> +       return bpf_copy_to_user(ubuf, tp_name, ulen, tp_len);
>  }
>
>  static const struct bpf_link_ops bpf_raw_tp_link_lops = {
> --
> 2.39.3
>

Re: [PATCH v6 bpf-next 08/11] bpf: Add bpf_perf_link_fill_common()

[Andrii Nakryiko]

On Wed, Jun 28, 2023 at 4:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> Add a new helper bpf_perf_link_fill_common(), which will be used by
> perf_link based tracepoint, kprobe and uprobe.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  kernel/bpf/syscall.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 4aa6e5776a04..72de91beabbc 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3364,6 +3364,40 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
>         kfree(perf_link);
>  }
>
> +static int bpf_perf_link_fill_common(const struct perf_event *event,
> +                                    char __user *uname, u32 ulen,
> +                                    u64 *probe_offset, u64 *probe_addr,
> +                                    u32 *fd_type)
> +{
> +       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) {

if buf is NULL, strlen above will crash, so you need to calculate len
inside this if branch


> +               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 const struct bpf_link_ops bpf_perf_link_lops = {
>         .release = bpf_perf_link_release,
>         .dealloc = bpf_perf_link_dealloc,
> --
> 2.39.3
>

Re: [PATCH v6 bpf-next 08/11] bpf: Add bpf_perf_link_fill_common()

[Yafang Shao]

On Fri, Jul 7, 2023 at 6:00 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Jun 28, 2023 at 4:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > Add a new helper bpf_perf_link_fill_common(), which will be used by
> > perf_link based tracepoint, kprobe and uprobe.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> >  kernel/bpf/syscall.c | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 4aa6e5776a04..72de91beabbc 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3364,6 +3364,40 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
> >         kfree(perf_link);
> >  }
> >
> > +static int bpf_perf_link_fill_common(const struct perf_event *event,
> > +                                    char __user *uname, u32 ulen,
> > +                                    u64 *probe_offset, u64 *probe_addr,
> > +                                    u32 *fd_type)
> > +{
> > +       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) {
>
> if buf is NULL, strlen above will crash, so you need to calculate len
> inside this if branch

Nice catch. Will fix it.

-- 
Regards
Yafang

Re: [PATCH v6 bpf-next 01/11] bpf: Support ->fill_link_info for kprobe_multi

[Yafang Shao]

On Fri, Jul 7, 2023 at 6:00 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Jun 28, 2023 at 4:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > With the addition of support for fill_link_info to the kprobe_multi link,
> > users will gain the ability to inspect it conveniently using the
> > `bpftool link show`. This enhancement provides valuable information to the
> > user, including the count of probed functions and their respective
> > addresses. It's important to note that if the kptr_restrict setting is not
> > permitted, the probed address will not be exposed, ensuring security.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
>
> documentation nit, but otherwise LGTM
>
> Also, looking at other patch where you introduce bpf_copy_user(),
> seems like we return -ENOSPC when user-provided memory is not big
> enough. So let's change E2BIG in this patch to ENOSPC?

Sure. Will change it.

>
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>

Thanks for your review.

>
> >  include/uapi/linux/bpf.h       |  5 +++++
> >  kernel/trace/bpf_trace.c       | 37 ++++++++++++++++++++++++++++++++++
> >  tools/include/uapi/linux/bpf.h |  5 +++++
> >  3 files changed, 47 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 60a9d59beeab..512ba3ba2ed3 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -6439,6 +6439,11 @@ struct bpf_link_info {
> >                         __s32 priority;
> >                         __u32 flags;
> >                 } netfilter;
> > +               struct {
> > +                       __aligned_u64 addrs; /* in/out: addresses buffer ptr */
>
> addrs field itself is not modified, the memory it points to is
> modified, so it's not really in/out per se, it is just a pointer to
> memory where kernel stores output data

Thanks for the explanation. Will change it.

>
> > +                       __u32 count;
>
> but count field itself is in/out, so please add a comment explicitly
> stating this

Will do it.


-- 
Regards
Yafang