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@xxxxxxxxxx>
---
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
