Re: [PATCH bpf-next 3/7] libbpf: add kprobe/uprobe attach API

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Thu, Jun 20, 2019 at 5:04 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/20, Andrii Nakryiko 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@fb.com>
> > ---
> >  tools/lib/bpf/libbpf.c   | 207 +++++++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf.h   |   8 ++
> >  tools/lib/bpf/libbpf.map |   2 +
> >  3 files changed, 217 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 2bb1fa008be3..11329e05530e 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -3969,6 +3969,213 @@ int bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
> >       return 0;
> >  }
> >
> > +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));
> > +     close(fd);
> > +     if (ret < 0) {
> > +             ret = -errno;
> Is -errno still valid here after a close(fd) above? Do we have any
> guarantee of errno preservation when we do another syscall?

Good catch! No, close() can change errno. Fixed. Also fixed for
parse_config_from_file below.

>
> > +             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));
> > +     close(fd);
> > +     if (ret < 0) {
> > +             ret = -errno;
> > +             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);
> > +}
> > +

<snip>