netdev - Re: [PATCH v4 bpf-next 3/5] libbpf: add auto-attach for uprobes based on section name

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 15 Mar 2022 21:33:20 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alan Maguire <alan.maguire@...cle.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, Martin Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        john fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Yucong Sun <sunyucong@...il.com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH v4 bpf-next 3/5] libbpf: add auto-attach for uprobes based
 on section name

On Fri, Mar 11, 2022 at 4:11 AM Alan Maguire <alan.maguire@...cle.com> wrote:
>
> Now that u[ret]probes can use name-based specification, it makes
> sense to add support for auto-attach based on SEC() definition.
> The format proposed is
>
>         SEC("u[ret]probe/prog:[raw_offset|[function_name[+offset]]")

nit: prog -> binary ? or prog -> path?

>
> For example, to trace malloc() in libc:
>
>         SEC("uprobe/libc.so.6:malloc")
>
> ...or to trace function foo2 in /usr/bin/foo:
>
>         SEC("uprobe//usr/bin/foo:foo2")
>
> Auto-attach is done for all tasks (pid -1).  prog can be an absolute
> path or simply a program/library name; in the latter case, we use
> PATH/LD_LIBRARY_PATH to resolve the full path, falling back to
> standard locations (/usr/bin:/usr/sbin or /usr/lib64:/usr/lib) if
> the file is not found via environment-variable specified locations.
>
> Signed-off-by: Alan Maguire <alan.maguire@...cle.com>
> ---
>  tools/lib/bpf/libbpf.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 66 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2b50b01..0dcbca8 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -8593,6 +8593,7 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
>  }
>
>  static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> +static int attach_uprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>  static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>  static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>  static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> @@ -8604,9 +8605,9 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
>         SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
>         SEC_DEF("sk_reuseport",         SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
>         SEC_DEF("kprobe/",              KPROBE, 0, SEC_NONE, attach_kprobe),
> -       SEC_DEF("uprobe/",              KPROBE, 0, SEC_NONE),
> +       SEC_DEF("uprobe/",              KPROBE, 0, SEC_NONE, attach_uprobe),
>         SEC_DEF("kretprobe/",           KPROBE, 0, SEC_NONE, attach_kprobe),
> -       SEC_DEF("uretprobe/",           KPROBE, 0, SEC_NONE),
> +       SEC_DEF("uretprobe/",           KPROBE, 0, SEC_NONE, attach_uprobe),
>         SEC_DEF("tc",                   SCHED_CLS, 0, SEC_NONE),
>         SEC_DEF("classifier",           SCHED_CLS, 0, SEC_NONE | SEC_SLOPPY_PFX | SEC_DEPRECATED),
>         SEC_DEF("action",               SCHED_ACT, 0, SEC_NONE | SEC_SLOPPY_PFX),
> @@ -10761,6 +10762,69 @@ struct bpf_link *bpf_program__attach_uprobe(const struct bpf_program *prog,
>         return bpf_program__attach_uprobe_opts(prog, pid, binary_path, func_offset, &opts);
>  }
>
> +/* Format of u[ret]probe section definition supporting auto-attach:
> + * u[ret]probe/prog:function[+offset]

same about prog

> + *
> + * prog can be an absolute/relative path or a filename; the latter is resolved to a
> + * full path via bpf_program__attach_uprobe_opts.
> + *
> + * Many uprobe programs do not avail of auto-attach, so we need to handle the

do not avail of? meaning "can't be auto-attached due to missing information"?

> + * case where the format is uprobe/myfunc by returning 0 with *link set to NULL
> + * to identify the case where auto-attach is not supported.

it's true that we supported SEC("uprobe/whatever") before and that's
not enough to auto-attach. But let's not drag this legacy "syntax"
forward. How about we check if LIBBPF_STRICT_SEC_NAME is set, and if
yes, then it should either be plain SEC("uprobe") or a proper full
form of SEC("uprobe/path:func...") that you are adding? libbpf
supports such case, you just need to change SEC_DEF definition to
uprobe/ -> uprobe+, which means that it is either SEC("uprobe") or
SEC("uprobe/<something>").

In legacy mode we just won't support auto-attach for uprobe.

Thoughts?

> + */
> +static int attach_uprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> +{
> +       DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
> +       char *func, *probe_name, *func_end;
> +       char *func_name, binary_path[512];
> +       unsigned long long raw_offset;
> +       size_t offset = 0;
> +       int n;
> +
> +       *link = NULL;
> +

[...]