| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEf4Bzbo4r9=VZ2kYaOsZa7HHvjXeEw4uWXhpjcUDvazOcKrzw@mail.gmail.com>
Date: Fri, 28 Jun 2019 12:59:17 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Song Liu <liu.song.a23@...il.com>
Cc: Andrii Nakryiko <andriin@...com>, Alexei Starovoitov <ast@...com>,
Daniel Borkmann <daniel@...earbox.net>,
Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
Stanislav Fomichev <sdf@...ichev.me>,
Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
On Fri, Jun 28, 2019 at 12:46 PM Song Liu <liu.song.a23@...il.com> wrote:
>
> On Thu, Jun 27, 2019 at 10:53 PM Andrii Nakryiko <andriin@...com> wrote:
> >
> > Add ability to attach to kernel and user probes and retprobes.
> > Implementation depends on perf event support for kprobes/uprobes.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@...com>
> > ---
> > tools/lib/bpf/libbpf.c | 213 +++++++++++++++++++++++++++++++++++++++
> > tools/lib/bpf/libbpf.h | 7 ++
> > tools/lib/bpf/libbpf.map | 2 +
> > 3 files changed, 222 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 606705f878ba..65d2fef41003 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4016,6 +4016,219 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> > return (struct bpf_link *)link;
> > }
> >
> > +static int parse_uint(const char *buf)
> > +{
> > + int ret;
> > +
> > + errno = 0;
> > + ret = (int)strtol(buf, NULL, 10);
> > + if (errno) {
> > + ret = -errno;
> > + pr_debug("failed to parse '%s' as unsigned int\n", buf);
> > + return ret;
> > + }
> > + if (ret < 0) {
> > + pr_debug("failed to parse '%s' as unsigned int\n", buf);
> > + return -EINVAL;
> > + }
> > + return ret;
> > +}
> > +
> > +static int parse_uint_from_file(const char* file)
> > +{
> > + char buf[STRERR_BUFSIZE];
> > + int fd, ret;
> > +
> > + fd = open(file, O_RDONLY);
> > + if (fd < 0) {
> > + ret = -errno;
> > + pr_debug("failed to open '%s': %s\n", file,
> > + libbpf_strerror_r(ret, buf, sizeof(buf)));
> > + return ret;
> > + }
> > + ret = read(fd, buf, sizeof(buf));
> > + ret = ret < 0 ? -errno : ret;
> > + close(fd);
> > + if (ret < 0) {
> > + pr_debug("failed to read '%s': %s\n", file,
> > + libbpf_strerror_r(ret, buf, sizeof(buf)));
> > + return ret;
> > + }
> > + if (ret == 0 || ret >= sizeof(buf)) {
> > + buf[sizeof(buf) - 1] = 0;
> > + pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> > + return -EINVAL;
> > + }
> > + return parse_uint(buf);
> > +}
> > +
> > +static int determine_kprobe_perf_type(void)
> > +{
> > + const char *file = "/sys/bus/event_source/devices/kprobe/type";
> > + return parse_uint_from_file(file);
> > +}
> > +
> > +static int determine_uprobe_perf_type(void)
> > +{
> > + const char *file = "/sys/bus/event_source/devices/uprobe/type";
> > + return parse_uint_from_file(file);
> > +}
> > +
> > +static int parse_config_from_file(const char *file)
> > +{
> > + char buf[STRERR_BUFSIZE];
> > + int fd, ret;
> > +
> > + fd = open(file, O_RDONLY);
> > + if (fd < 0) {
> > + ret = -errno;
> > + pr_debug("failed to open '%s': %s\n", file,
> > + libbpf_strerror_r(ret, buf, sizeof(buf)));
> > + return ret;
> > + }
> > + ret = read(fd, buf, sizeof(buf));
> > + ret = ret < 0 ? -errno : ret;
> > + close(fd);
> > + if (ret < 0) {
> > + pr_debug("failed to read '%s': %s\n", file,
> > + libbpf_strerror_r(ret, buf, sizeof(buf)));
> > + return ret;
> > + }
> > + if (ret == 0 || ret >= sizeof(buf)) {
> > + buf[sizeof(buf) - 1] = 0;
> > + pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> > + return -EINVAL;
> > + }
> > + if (strncmp(buf, "config:", 7)) {
> > + pr_debug("expected 'config:' prefix, found '%s'\n", buf);
> > + return -EINVAL;
> > + }
> > + return parse_uint(buf + 7);
> > +}
> > +
> > +static int determine_kprobe_retprobe_bit(void)
> > +{
> > + const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> > + return parse_config_from_file(file);
> > +}
> > +
> > +static int determine_uprobe_retprobe_bit(void)
> > +{
> > + const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> > + return parse_config_from_file(file);
> > +}
>
> Can we do the above with fscanf? Would that be easier?
It would be less code, but also less strict semantics. E.g., fscanf
would happily leave out any garbage after number (e.g., 123blablabla,
would still parse). Also, from brief googling, fscanf doesn't handle
overflows well.
So I guess I'd vote for this more verbose, but also more strict
checking, unless you insist on fscanf.
>
> > +
> > +static int perf_event_open_probe(bool uprobe, bool retprobe, const char* name,
> > + uint64_t offset, int pid)
> > +{
> > + struct perf_event_attr attr = {};
> > + char errmsg[STRERR_BUFSIZE];
> > + int type, pfd, err;
> > +
> > + type = uprobe ? determine_uprobe_perf_type()
> > + : determine_kprobe_perf_type();
> > + if (type < 0) {
> > + pr_warning("failed to determine %s perf type: %s\n",
> > + uprobe ? "uprobe" : "kprobe",
> > + libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> > + return type;
> > + }
> > + if (retprobe) {
> > + int bit = uprobe ? determine_uprobe_retprobe_bit()
> > + : determine_kprobe_retprobe_bit();
> > +
> > + if (bit < 0) {
> > + pr_warning("failed to determine %s retprobe bit: %s\n",
> > + uprobe ? "uprobe" : "kprobe",
> > + libbpf_strerror_r(bit, errmsg,
> > + sizeof(errmsg)));
> > + return bit;
> > + }
> > + attr.config |= 1 << bit;
> > + }
> > + attr.size = sizeof(attr);
> > + attr.type = type;
> > + attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> > + attr.config2 = offset; /* kprobe_addr or probe_offset */
> > +
> > + /* pid filter is meaningful only for uprobes */
> > + 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_warning("%s perf_event_open() failed: %s\n",
> > + uprobe ? "uprobe" : "kprobe",
> > + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
>
> We have another warning in bpf_program__attach_[k|u]probe(). I guess
> we can remove this one here.
This points specifically to perf_event_open() failing versus other
possible failures. Messages in attach_{k,u}probe won't have that, they
will repeat more generic "failed to attach" message. Believe me, if
something goes wrong in libbpf, I'd rather have too much logging than
too little :)
>
> > + return err;
> > + }
> > + return pfd;
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
> > + bool retprobe,
> > + const char *func_name)
> > +{
> > + char errmsg[STRERR_BUFSIZE];
> > + struct bpf_link *link;
> > + int pfd, err;
> > +
> > + pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> > + 0 /* offset */, -1 /* pid */);
> > + if (pfd < 0) {
> > + pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> > + bpf_program__title(prog, false),
> > + retprobe ? "kretprobe" : "kprobe", func_name,
> > + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> > + return ERR_PTR(pfd);
> > + }
> > + link = bpf_program__attach_perf_event(prog, pfd);
> > + if (IS_ERR(link)) {
> > + close(pfd);
> > + err = PTR_ERR(link);
> > + pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> > + bpf_program__title(prog, false),
> > + retprobe ? "kretprobe" : "kprobe", func_name,
> > + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > + return link;
> > + }
> > + return link;
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> > + bool retprobe, pid_t pid,
> > + const char *binary_path,
> > + size_t func_offset)
> > +{
> > + char errmsg[STRERR_BUFSIZE];
> > + struct bpf_link *link;
> > + int pfd, err;
> > +
> > + pfd = perf_event_open_probe(true /* uprobe */, retprobe,
> > + binary_path, func_offset, pid);
> > + if (pfd < 0) {
> > + pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
> > + bpf_program__title(prog, false),
> > + retprobe ? "uretprobe" : "uprobe",
> > + binary_path, func_offset,
> > + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> > + return ERR_PTR(pfd);
> > + }
> > + link = bpf_program__attach_perf_event(prog, pfd);
> > + if (IS_ERR(link)) {
> > + close(pfd);
> > + err = PTR_ERR(link);
> > + pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
> > + bpf_program__title(prog, false),
> > + retprobe ? "uretprobe" : "uprobe",
> > + binary_path, func_offset,
> > + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > + return link;
> > + }
> > + return link;
> > +}
> > +
> > enum bpf_perf_event_ret
> > bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> > void **copy_mem, size_t *copy_size,
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 1bf66c4a9330..bd767cc11967 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> >
> > LIBBPF_API struct bpf_link *
> > bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> > +LIBBPF_API struct bpf_link *
> > +bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> > + const char *func_name);
> > +LIBBPF_API struct bpf_link *
> > +bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> > + pid_t pid, const char *binary_path,
> > + size_t func_offset);
> >
> > struct bpf_insn;
> >
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 756f5aa802e9..57a40fb60718 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
> > global:
> > bpf_link__destroy;
> > bpf_object__load_xattr;
> > + bpf_program__attach_kprobe;
> > bpf_program__attach_perf_event;
> > + bpf_program__attach_uprobe;
> > btf_dump__dump_type;
> > btf_dump__free;
> > btf_dump__new;
> > --
> > 2.17.1
> >
Powered by blists - more mailing lists