* [PATCH bpf-next 1/4] libbpf: fix memory leak in legacy kprobe attach logic
2021-09-20 23:16 [PATCH bpf-next 0/4] libbpf: add legacy uprobe support Andrii Nakryiko
@ 2021-09-20 23:16 ` Andrii Nakryiko
2021-09-20 23:16 ` [PATCH bpf-next 2/4] selftests/bpf: adopt attach_probe selftest to work on old kernels Andrii Nakryiko
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2021-09-20 23:16 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next 2/4] selftests/bpf: adopt attach_probe selftest to work on old kernels
2021-09-20 23:16 [PATCH bpf-next 0/4] libbpf: add legacy uprobe support Andrii Nakryiko
2021-09-20 23:16 ` [PATCH bpf-next 1/4] libbpf: fix memory leak in legacy kprobe attach logic Andrii Nakryiko
@ 2021-09-20 23:16 ` Andrii Nakryiko
2021-09-21 20:34 ` Alexei Starovoitov
2021-09-20 23:16 ` [PATCH bpf-next 3/4] libbpf: refactor and simplify legacy kprobe code Andrii Nakryiko
2021-09-20 23:16 ` [PATCH bpf-next 4/4] libbpf: add legacy uprobe attaching support Andrii Nakryiko
3 siblings, 1 reply; 9+ messages in thread
From: Andrii Nakryiko @ 2021-09-20 23:16 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/4] selftests/bpf: adopt attach_probe selftest to work on old kernels
2021-09-20 23:16 ` [PATCH bpf-next 2/4] selftests/bpf: adopt attach_probe selftest to work on old kernels Andrii Nakryiko
@ 2021-09-21 20:34 ` Alexei Starovoitov
2021-09-21 20:46 ` Andrii Nakryiko
0 siblings, 1 reply; 9+ messages in thread
From: Alexei Starovoitov @ 2021-09-21 20:34 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
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?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/4] selftests/bpf: adopt attach_probe selftest to work on old kernels
2021-09-21 20:34 ` Alexei Starovoitov
@ 2021-09-21 20:46 ` Andrii Nakryiko
2021-09-21 20:48 ` Alexei Starovoitov
0 siblings, 1 reply; 9+ messages in thread
From: Andrii Nakryiko @ 2021-09-21 20:46 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/4] selftests/bpf: adopt attach_probe selftest to work on old kernels
2021-09-21 20:46 ` Andrii Nakryiko
@ 2021-09-21 20:48 ` Alexei Starovoitov
2021-09-21 20:53 ` Andrii Nakryiko
0 siblings, 1 reply; 9+ messages in thread
From: Alexei Starovoitov @ 2021-09-21 20:48 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
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?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/4] selftests/bpf: adopt attach_probe selftest to work on old kernels
2021-09-21 20:48 ` Alexei Starovoitov
@ 2021-09-21 20:53 ` Andrii Nakryiko
0 siblings, 0 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2021-09-21 20:53 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next 3/4] libbpf: refactor and simplify legacy kprobe code
2021-09-20 23:16 [PATCH bpf-next 0/4] libbpf: add legacy uprobe support Andrii Nakryiko
2021-09-20 23:16 ` [PATCH bpf-next 1/4] libbpf: fix memory leak in legacy kprobe attach logic Andrii Nakryiko
2021-09-20 23:16 ` [PATCH bpf-next 2/4] selftests/bpf: adopt attach_probe selftest to work on old kernels Andrii Nakryiko
@ 2021-09-20 23:16 ` Andrii Nakryiko
2021-09-20 23:16 ` [PATCH bpf-next 4/4] libbpf: add legacy uprobe attaching support Andrii Nakryiko
3 siblings, 0 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2021-09-20 23:16 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next 4/4] libbpf: add legacy uprobe attaching support
2021-09-20 23:16 [PATCH bpf-next 0/4] libbpf: add legacy uprobe support Andrii Nakryiko
` (2 preceding siblings ...)
2021-09-20 23:16 ` [PATCH bpf-next 3/4] libbpf: refactor and simplify legacy kprobe code Andrii Nakryiko
@ 2021-09-20 23:16 ` Andrii Nakryiko
3 siblings, 0 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2021-09-20 23:16 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
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
^ permalink raw reply related [flat|nested] 9+ messages in thread