Implement libbpf support for attaching uprobes/uretprobes using legacy tracefs interfaces. This is a logical complement to recently landed legacy kprobe support ([0]). This patch refactors existing legacy kprobe code to be more uniform with uprobe code as well, making the logic easier to compare and follow. This patch set also fixes two bugs recently found by Coverity in legacy kprobe handling code, and thus subsumes previously submitted two patches ([1]): original patch #1 is kept as is, while original patch #2 was dropped because patch #3 of the current series refactors and fixes affected code. [0] https://patchwork.kernel.org/project/netdevbpf/patch/20210912064844.3181742-1-rafaeldtinoco@gmail.com/ [1] https://patchwork.kernel.org/project/netdevbpf/list/?series=549977&state=* Andrii Nakryiko (4): libbpf: fix memory leak in legacy kprobe attach logic selftests/bpf: adopt attach_probe selftest to work on old kernels libbpf: refactor and simplify legacy kprobe code libbpf: add legacy uprobe attaching support tools/lib/bpf/libbpf.c | 297 +++++++++++++----- tools/lib/bpf/libbpf.h | 2 +- .../selftests/bpf/prog_tests/attach_probe.c | 16 +- 3 files changed, 228 insertions(+), 87 deletions(-) -- 2.30.2
In some error scenarios legacy_probe string won't be free()'d. Fix this. This was reported by Coverity static analysis. Fixes: ca304b40c20d ("libbpf: Introduce legacy kprobe events support") Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/lib/bpf/libbpf.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index da65a1666a5e..6d2f12db6034 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -9365,10 +9365,11 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, offset, -1 /* pid */); } if (pfd < 0) { + err = pfd; pr_warn("prog '%s': failed to create %s '%s' perf event: %s\n", prog->name, retprobe ? "kretprobe" : "kprobe", func_name, - libbpf_strerror_r(pfd, errmsg, sizeof(errmsg))); - return libbpf_err_ptr(pfd); + libbpf_strerror_r(err, errmsg, sizeof(errmsg))); + goto err_out; } link = bpf_program__attach_perf_event_opts(prog, pfd, &pe_opts); err = libbpf_get_error(link); @@ -9377,7 +9378,7 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, pr_warn("prog '%s': failed to attach to %s '%s': %s\n", prog->name, retprobe ? "kretprobe" : "kprobe", func_name, libbpf_strerror_r(err, errmsg, sizeof(errmsg))); - return libbpf_err_ptr(err); + goto err_out; } if (legacy) { struct bpf_link_perf *perf_link = container_of(link, struct bpf_link_perf, link); @@ -9387,6 +9388,9 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, } return link; +err_out: + free(legacy_probe); + return libbpf_err_ptr(err); } struct bpf_link *bpf_program__attach_kprobe(const struct bpf_program *prog, -- 2.30.2
Make sure to not use ref_ctr_off feature when running on old kernels that don't support this feature. This allows to test libbpf's legacy kprobe and uprobe logic on old kernels. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c index bf307bb9e446..cbd6b6175d5c 100644 --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c @@ -14,6 +14,12 @@ void test_attach_probe(void) struct test_attach_probe* skel; size_t uprobe_offset; ssize_t base_addr, ref_ctr_offset; + bool legacy; + + /* check is new-style kprobe/uprobe API is supported */ + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0; + + legacy = true; base_addr = get_base_addr(); if (CHECK(base_addr < 0, "get_base_addr", @@ -45,10 +51,11 @@ void test_attach_probe(void) goto cleanup; skel->links.handle_kretprobe = kretprobe_link; - ASSERT_EQ(uprobe_ref_ctr, 0, "uprobe_ref_ctr_before"); + if (!legacy) + ASSERT_EQ(uprobe_ref_ctr, 0, "uprobe_ref_ctr_before"); uprobe_opts.retprobe = false; - uprobe_opts.ref_ctr_offset = ref_ctr_offset; + uprobe_opts.ref_ctr_offset = legacy ? 0 : ref_ctr_offset; uprobe_link = bpf_program__attach_uprobe_opts(skel->progs.handle_uprobe, 0 /* self pid */, "/proc/self/exe", @@ -58,11 +65,12 @@ void test_attach_probe(void) goto cleanup; skel->links.handle_uprobe = uprobe_link; - ASSERT_GT(uprobe_ref_ctr, 0, "uprobe_ref_ctr_after"); + if (!legacy) + ASSERT_GT(uprobe_ref_ctr, 0, "uprobe_ref_ctr_after"); /* if uprobe uses ref_ctr, uretprobe has to use ref_ctr as well */ uprobe_opts.retprobe = true; - uprobe_opts.ref_ctr_offset = ref_ctr_offset; + uprobe_opts.ref_ctr_offset = legacy ? 0 : ref_ctr_offset; uretprobe_link = bpf_program__attach_uprobe_opts(skel->progs.handle_uretprobe, -1 /* any pid */, "/proc/self/exe", -- 2.30.2
Refactor legacy kprobe handling code to follow the same logic as uprobe legacy logic added in the next patchs: - add append_to_file() helper that makes it simpler to work with tracefs file-based interface for creating and deleting probes; - move out probe/event name generation outside of the code that adds/removes it, which simplifies bookkeeping significantly; - change the probe name format to start with "libbpf_" prefix and include offset within kernel function; - switch 'unsigned long' to 'size_t' for specifying kprobe offsets, which is consistent with how uprobes define that, simplifies printf()-ing internally, and also avoids unnecessary complications on architectures where sizeof(long) != sizeof(void *). This patch also implicitly fixes the problem with invalid open() error handling present in poke_kprobe_events(), which (the function) this patch removes. Fixes: ca304b40c20d ("libbpf: Introduce legacy kprobe events support") Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/lib/bpf/libbpf.c | 159 ++++++++++++++++++++++------------------- tools/lib/bpf/libbpf.h | 2 +- 2 files changed, 88 insertions(+), 73 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 6d2f12db6034..aa842f0721cb 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -9011,59 +9011,17 @@ int bpf_link__unpin(struct bpf_link *link) return 0; } -static int poke_kprobe_events(bool add, const char *name, bool retprobe, uint64_t offset) -{ - int fd, ret = 0; - pid_t p = getpid(); - char cmd[260], probename[128], probefunc[128]; - const char *file = "/sys/kernel/debug/tracing/kprobe_events"; - - if (retprobe) - snprintf(probename, sizeof(probename), "kretprobes/%s_libbpf_%u", name, p); - else - snprintf(probename, sizeof(probename), "kprobes/%s_libbpf_%u", name, p); - - if (offset) - snprintf(probefunc, sizeof(probefunc), "%s+%zu", name, (size_t)offset); - - if (add) { - snprintf(cmd, sizeof(cmd), "%c:%s %s", - retprobe ? 'r' : 'p', - probename, - offset ? probefunc : name); - } else { - snprintf(cmd, sizeof(cmd), "-:%s", probename); - } - - fd = open(file, O_WRONLY | O_APPEND, 0); - if (!fd) - return -errno; - ret = write(fd, cmd, strlen(cmd)); - if (ret < 0) - ret = -errno; - close(fd); - - return ret; -} - -static inline int add_kprobe_event_legacy(const char *name, bool retprobe, uint64_t offset) -{ - return poke_kprobe_events(true, name, retprobe, offset); -} - -static inline int remove_kprobe_event_legacy(const char *name, bool retprobe) -{ - return poke_kprobe_events(false, name, retprobe, 0); -} - struct bpf_link_perf { struct bpf_link link; int perf_event_fd; /* legacy kprobe support: keep track of probe identifier and type */ char *legacy_probe_name; + bool legacy_is_kprobe; bool legacy_is_retprobe; }; +static int remove_kprobe_event_legacy(const char *probe_name, bool retprobe); + static int bpf_link_perf_detach(struct bpf_link *link) { struct bpf_link_perf *perf_link = container_of(link, struct bpf_link_perf, link); @@ -9077,9 +9035,12 @@ static int bpf_link_perf_detach(struct bpf_link *link) close(link->fd); /* legacy kprobe needs to be removed after perf event fd closure */ - if (perf_link->legacy_probe_name) - err = remove_kprobe_event_legacy(perf_link->legacy_probe_name, - perf_link->legacy_is_retprobe); + if (perf_link->legacy_probe_name) { + if (perf_link->legacy_is_kprobe) { + err = remove_kprobe_event_legacy(perf_link->legacy_probe_name, + perf_link->legacy_is_retprobe); + } + } return err; } @@ -9202,18 +9163,6 @@ static int parse_uint_from_file(const char *file, const char *fmt) return ret; } -static int determine_kprobe_perf_type_legacy(const char *func_name, bool is_retprobe) -{ - char file[192]; - - snprintf(file, sizeof(file), - "/sys/kernel/debug/tracing/events/%s/%s_libbpf_%d/id", - is_retprobe ? "kretprobes" : "kprobes", - func_name, getpid()); - - return parse_uint_from_file(file, "%d\n"); -} - static int determine_kprobe_perf_type(void) { const char *file = "/sys/bus/event_source/devices/kprobe/type"; @@ -9296,21 +9245,79 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name, return pfd; } -static int perf_event_kprobe_open_legacy(bool retprobe, const char *name, uint64_t offset, int pid) +static int append_to_file(const char *file, const char *fmt, ...) +{ + int fd, n, err = 0; + va_list ap; + + fd = open(file, O_WRONLY | O_APPEND, 0); + if (fd < 0) + return -errno; + + va_start(ap, fmt); + n = vdprintf(fd, fmt, ap); + va_end(ap); + + if (n < 0) + err = -errno; + + close(fd); + return err; +} + +static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz, + const char *kfunc_name, size_t offset) +{ + snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx", getpid(), kfunc_name, offset); +} + +static int add_kprobe_event_legacy(const char *probe_name, bool retprobe, + const char *kfunc_name, size_t offset) +{ + const char *file = "/sys/kernel/debug/tracing/kprobe_events"; + + return append_to_file(file, "%c:%s/%s %s+0x%zx", + retprobe ? 'r' : 'p', + retprobe ? "kretprobes" : "kprobes", + probe_name, kfunc_name, offset); +} + +static int remove_kprobe_event_legacy(const char *probe_name, bool retprobe) +{ + const char *file = "/sys/kernel/debug/tracing/kprobe_events"; + + return append_to_file(file, "-:%s/%s", retprobe ? "kretprobes" : "kprobes", probe_name); +} + +static int determine_kprobe_perf_type_legacy(const char *probe_name, bool retprobe) +{ + char file[256]; + + snprintf(file, sizeof(file), + "/sys/kernel/debug/tracing/events/%s/%s/id", + retprobe ? "kretprobes" : "kprobes", probe_name); + + return parse_uint_from_file(file, "%d\n"); +} + +static int perf_event_kprobe_open_legacy(const char *probe_name, bool retprobe, + const char *kfunc_name, size_t offset, int pid) { struct perf_event_attr attr = {}; char errmsg[STRERR_BUFSIZE]; int type, pfd, err; - err = add_kprobe_event_legacy(name, retprobe, offset); + err = add_kprobe_event_legacy(probe_name, retprobe, kfunc_name, offset); if (err < 0) { - pr_warn("failed to add legacy kprobe event: %s\n", + pr_warn("failed to add legacy kprobe event for '%s+0x%zx': %s\n", + kfunc_name, offset, libbpf_strerror_r(err, errmsg, sizeof(errmsg))); return err; } - type = determine_kprobe_perf_type_legacy(name, retprobe); + type = determine_kprobe_perf_type_legacy(probe_name, retprobe); if (type < 0) { - pr_warn("failed to determine legacy kprobe event id: %s\n", + pr_warn("failed to determine legacy kprobe event id for '%s+0x%zx': %s\n", + kfunc_name, offset, libbpf_strerror_r(type, errmsg, sizeof(errmsg))); return type; } @@ -9340,7 +9347,7 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, char errmsg[STRERR_BUFSIZE]; char *legacy_probe = NULL; struct bpf_link *link; - unsigned long offset; + size_t offset; bool retprobe, legacy; int pfd, err; @@ -9357,17 +9364,23 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, func_name, offset, -1 /* pid */, 0 /* ref_ctr_off */); } else { + char probe_name[256]; + + gen_kprobe_legacy_event_name(probe_name, sizeof(probe_name), + func_name, offset); + legacy_probe = strdup(func_name); if (!legacy_probe) return libbpf_err_ptr(-ENOMEM); - pfd = perf_event_kprobe_open_legacy(retprobe, func_name, + pfd = perf_event_kprobe_open_legacy(legacy_probe, retprobe, func_name, offset, -1 /* pid */); } if (pfd < 0) { - err = pfd; - pr_warn("prog '%s': failed to create %s '%s' perf event: %s\n", - prog->name, retprobe ? "kretprobe" : "kprobe", func_name, + err = -errno; + pr_warn("prog '%s': failed to create %s '%s+0x%zx' perf event: %s\n", + prog->name, retprobe ? "kretprobe" : "kprobe", + func_name, offset, libbpf_strerror_r(err, errmsg, sizeof(errmsg))); goto err_out; } @@ -9375,8 +9388,9 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, err = libbpf_get_error(link); if (err) { close(pfd); - pr_warn("prog '%s': failed to attach to %s '%s': %s\n", - prog->name, retprobe ? "kretprobe" : "kprobe", func_name, + pr_warn("prog '%s': failed to attach to %s '%s+0x%zx': %s\n", + prog->name, retprobe ? "kretprobe" : "kprobe", + func_name, offset, libbpf_strerror_r(err, errmsg, sizeof(errmsg))); goto err_out; } @@ -9384,6 +9398,7 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, struct bpf_link_perf *perf_link = container_of(link, struct bpf_link_perf, link); perf_link->legacy_probe_name = legacy_probe; + perf_link->legacy_is_kprobe = true; perf_link->legacy_is_retprobe = retprobe; } diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index c90e3d79e72c..262eb3038e83 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -269,7 +269,7 @@ struct bpf_kprobe_opts { /* custom user-provided value fetchable through bpf_get_attach_cookie() */ __u64 bpf_cookie; /* function's offset to install kprobe to */ - unsigned long offset; + size_t offset; /* kprobe is return probe */ bool retprobe; size_t :0; -- 2.30.2
Similarly to recently added legacy kprobe attach interface support through tracefs, support attaching uprobes using the legacy interface if host kernel doesn't support newer FD-based interface. For uprobes event name consists of "libbpf_" prefix, PID, sanitized binary path and offset within that binary. Structuraly the code is aligned with kprobe logic refactoring in previous patch. struct bpf_link_perf is re-used and all the same legacy_probe_name and legacy_is_retprobe fields are used to ensure proper cleanup on bpf_link__destroy(). Users should be aware, though, that on old kernels which don't support FD-based interface for kprobe/uprobe attachment, if the application crashes before bpf_link__destroy() is called, uprobe legacy events will be left in tracefs. This is the same limitation as with legacy kprobe interfaces. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/lib/bpf/libbpf.c | 130 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 122 insertions(+), 8 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index aa842f0721cb..ef5db34bf913 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -9021,6 +9021,7 @@ struct bpf_link_perf { }; static int remove_kprobe_event_legacy(const char *probe_name, bool retprobe); +static int remove_uprobe_event_legacy(const char *probe_name, bool retprobe); static int bpf_link_perf_detach(struct bpf_link *link) { @@ -9034,11 +9035,14 @@ static int bpf_link_perf_detach(struct bpf_link *link) close(perf_link->perf_event_fd); close(link->fd); - /* legacy kprobe needs to be removed after perf event fd closure */ + /* legacy uprobe/kprobe needs to be removed after perf event fd closure */ if (perf_link->legacy_probe_name) { if (perf_link->legacy_is_kprobe) { err = remove_kprobe_event_legacy(perf_link->legacy_probe_name, perf_link->legacy_is_retprobe); + } else { + err = remove_uprobe_event_legacy(perf_link->legacy_probe_name, + perf_link->legacy_is_retprobe); } } @@ -9450,17 +9454,96 @@ static struct bpf_link *attach_kprobe(const struct bpf_program *prog) return link; } +static void gen_uprobe_legacy_event_name(char *buf, size_t buf_sz, + const char *binary_path, uint64_t offset) +{ + int i; + + snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx", getpid(), binary_path, (size_t)offset); + + /* sanitize binary_path in the probe name */ + for (i = 0; buf[i]; i++) { + if (!isalnum(buf[i])) + buf[i] = '_'; + } +} + +static inline int add_uprobe_event_legacy(const char *probe_name, bool retprobe, + const char *binary_path, size_t offset) +{ + const char *file = "/sys/kernel/debug/tracing/uprobe_events"; + + return append_to_file(file, "%c:%s/%s %s:0x%zx", + retprobe ? 'r' : 'p', + retprobe ? "uretprobes" : "uprobes", + probe_name, binary_path, offset); +} + +static inline int remove_uprobe_event_legacy(const char *probe_name, bool retprobe) +{ + const char *file = "/sys/kernel/debug/tracing/uprobe_events"; + + return append_to_file(file, "-:%s/%s", retprobe ? "uretprobes" : "uprobes", probe_name); +} + +static int determine_uprobe_perf_type_legacy(const char *probe_name, bool retprobe) +{ + char file[512]; + + snprintf(file, sizeof(file), + "/sys/kernel/debug/tracing/events/%s/%s/id", + retprobe ? "uretprobes" : "uprobes", probe_name); + + return parse_uint_from_file(file, "%d\n"); +} + +static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe, + const char *binary_path, size_t offset, int pid) +{ + struct perf_event_attr attr; + int type, pfd, err; + + err = add_uprobe_event_legacy(probe_name, retprobe, binary_path, offset); + if (err < 0) { + pr_warn("failed to add legacy uprobe event for %s:0x%zx: %d\n", + binary_path, (size_t)offset, err); + return err; + } + type = determine_uprobe_perf_type_legacy(probe_name, retprobe); + if (type < 0) { + pr_warn("failed to determine legacy uprobe event id for %s:0x%zx: %d\n", + binary_path, offset, err); + return type; + } + + memset(&attr, 0, sizeof(attr)); + attr.size = sizeof(attr); + attr.config = type; + attr.type = PERF_TYPE_TRACEPOINT; + + pfd = syscall(__NR_perf_event_open, &attr, + pid < 0 ? -1 : pid, /* pid */ + pid == -1 ? 0 : -1, /* cpu */ + -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC); + if (pfd < 0) { + err = -errno; + pr_warn("legacy uprobe perf_event_open() failed: %d\n", err); + return err; + } + return pfd; +} + LIBBPF_API struct bpf_link * bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid, const char *binary_path, size_t func_offset, const struct bpf_uprobe_opts *opts) { DECLARE_LIBBPF_OPTS(bpf_perf_event_opts, pe_opts); - char errmsg[STRERR_BUFSIZE]; + char errmsg[STRERR_BUFSIZE], *legacy_probe = NULL; struct bpf_link *link; size_t ref_ctr_off; int pfd, err; - bool retprobe; + bool retprobe, legacy; if (!OPTS_VALID(opts, bpf_uprobe_opts)) return libbpf_err_ptr(-EINVAL); @@ -9469,15 +9552,35 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid, ref_ctr_off = OPTS_GET(opts, ref_ctr_offset, 0); pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0); - pfd = perf_event_open_probe(true /* uprobe */, retprobe, binary_path, - func_offset, pid, ref_ctr_off); + legacy = determine_uprobe_perf_type() < 0; + if (!legacy) { + pfd = perf_event_open_probe(true /* uprobe */, retprobe, binary_path, + func_offset, pid, ref_ctr_off); + } else { + char probe_name[512]; + + if (ref_ctr_off) + return libbpf_err_ptr(-EINVAL); + + gen_uprobe_legacy_event_name(probe_name, sizeof(probe_name), + binary_path, func_offset); + + legacy_probe = strdup(probe_name); + if (!legacy_probe) + return libbpf_err_ptr(-ENOMEM); + + pfd = perf_event_uprobe_open_legacy(legacy_probe, retprobe, + binary_path, func_offset, pid); + } if (pfd < 0) { + err = -errno; pr_warn("prog '%s': failed to create %s '%s:0x%zx' perf event: %s\n", prog->name, retprobe ? "uretprobe" : "uprobe", binary_path, func_offset, - libbpf_strerror_r(pfd, errmsg, sizeof(errmsg))); - return libbpf_err_ptr(pfd); + libbpf_strerror_r(err, errmsg, sizeof(errmsg))); + goto err_out; } + link = bpf_program__attach_perf_event_opts(prog, pfd, &pe_opts); err = libbpf_get_error(link); if (err) { @@ -9486,9 +9589,20 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid, prog->name, retprobe ? "uretprobe" : "uprobe", binary_path, func_offset, libbpf_strerror_r(err, errmsg, sizeof(errmsg))); - return libbpf_err_ptr(err); + goto err_out; + } + if (legacy) { + struct bpf_link_perf *perf_link = container_of(link, struct bpf_link_perf, link); + + perf_link->legacy_probe_name = legacy_probe; + perf_link->legacy_is_kprobe = false; + perf_link->legacy_is_retprobe = retprobe; } return link; +err_out: + free(legacy_probe); + return libbpf_err_ptr(err); + } struct bpf_link *bpf_program__attach_uprobe(const struct bpf_program *prog, -- 2.30.2
On Mon, Sep 20, 2021 at 4:18 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Make sure to not use ref_ctr_off feature when running on old kernels
> that don't support this feature. This allows to test libbpf's legacy
> kprobe and uprobe logic on old kernels.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> index bf307bb9e446..cbd6b6175d5c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -14,6 +14,12 @@ void test_attach_probe(void)
> struct test_attach_probe* skel;
> size_t uprobe_offset;
> ssize_t base_addr, ref_ctr_offset;
> + bool legacy;
> +
> + /* check is new-style kprobe/uprobe API is supported */
> + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0;
> +
> + legacy = true;
What is the idea of the above?
One of them is a leftover?
On Tue, Sep 21, 2021 at 1:34 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Sep 20, 2021 at 4:18 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> >
> > Make sure to not use ref_ctr_off feature when running on old kernels
> > that don't support this feature. This allows to test libbpf's legacy
> > kprobe and uprobe logic on old kernels.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++----
> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > index bf307bb9e446..cbd6b6175d5c 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > @@ -14,6 +14,12 @@ void test_attach_probe(void)
> > struct test_attach_probe* skel;
> > size_t uprobe_offset;
> > ssize_t base_addr, ref_ctr_offset;
> > + bool legacy;
> > +
> > + /* check is new-style kprobe/uprobe API is supported */
> > + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0;
> > +
> > + legacy = true;
>
> What is the idea of the above?
> One of them is a leftover?
Oh, sorry, `legacy = true` was me locally testing, forgot to remove
that. This will be properly tested in libbpf CI where we have 4.9
kernel, I was just trying to simulate this locally on modern kernel.
I'll re-submit with this removed.
On Tue, Sep 21, 2021 at 1:47 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Sep 21, 2021 at 1:34 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Mon, Sep 20, 2021 at 4:18 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> > >
> > > Make sure to not use ref_ctr_off feature when running on old kernels
> > > that don't support this feature. This allows to test libbpf's legacy
> > > kprobe and uprobe logic on old kernels.
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > ---
> > > .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++----
> > > 1 file changed, 12 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > > index bf307bb9e446..cbd6b6175d5c 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > > @@ -14,6 +14,12 @@ void test_attach_probe(void)
> > > struct test_attach_probe* skel;
> > > size_t uprobe_offset;
> > > ssize_t base_addr, ref_ctr_offset;
> > > + bool legacy;
> > > +
> > > + /* check is new-style kprobe/uprobe API is supported */
> > > + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0;
> > > +
> > > + legacy = true;
> >
> > What is the idea of the above?
> > One of them is a leftover?
>
> Oh, sorry, `legacy = true` was me locally testing, forgot to remove
> that. This will be properly tested in libbpf CI where we have 4.9
> kernel, I was just trying to simulate this locally on modern kernel.
> I'll re-submit with this removed.
Got it.
Could you explain how access() works to detect it?
On Tue, Sep 21, 2021 at 1:48 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Sep 21, 2021 at 1:47 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Tue, Sep 21, 2021 at 1:34 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Mon, Sep 20, 2021 at 4:18 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> > > >
> > > > Make sure to not use ref_ctr_off feature when running on old kernels
> > > > that don't support this feature. This allows to test libbpf's legacy
> > > > kprobe and uprobe logic on old kernels.
> > > >
> > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > > ---
> > > > .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++----
> > > > 1 file changed, 12 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > > > index bf307bb9e446..cbd6b6175d5c 100644
> > > > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > > > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > > > @@ -14,6 +14,12 @@ void test_attach_probe(void)
> > > > struct test_attach_probe* skel;
> > > > size_t uprobe_offset;
> > > > ssize_t base_addr, ref_ctr_offset;
> > > > + bool legacy;
> > > > +
> > > > + /* check is new-style kprobe/uprobe API is supported */
> > > > + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0;
> > > > +
> > > > + legacy = true;
> > >
> > > What is the idea of the above?
> > > One of them is a leftover?
> >
> > Oh, sorry, `legacy = true` was me locally testing, forgot to remove
> > that. This will be properly tested in libbpf CI where we have 4.9
> > kernel, I was just trying to simulate this locally on modern kernel.
> > I'll re-submit with this removed.
>
> Got it.
> Could you explain how access() works to detect it?
Yeah, I'll expand the comment with this as well. The gist is that if
/sys/bus/event_source/devices/kprobe/type exists in the system, then
new FD-based kprobe attachment through perf system is supported,
because that file exposes a magic number that's passed to
perf_event_open() to create a kprobe event. So if access() is
successful, then it's a new enough kernel. Having said that, this is a
kprobe-specific file, while uprobe has its own (under
.../uprobe/type). Given we actually care about uprobe features for
this legacy check, I'll switch to an uprobe-specific one. But the idea
is the same.