netdev - Re: [PATCH v5 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:   Mon, 4 Apr 2022 22:43:46 +0100 (IST)
From:   Alan Maguire <alan.maguire@...cle.com>
To:     Ilya Leoshkevich <iii@...ux.ibm.com>
cc:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Alan Maguire <alan.maguire@...cle.com>,
        Andrii Nakryiko <andrii@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...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 v5 bpf-next 3/5] libbpf: add auto-attach for uprobes
 based on section name

On Mon, 4 Apr 2022, Ilya Leoshkevich wrote:

> On Sun, 2022-04-03 at 21:46 -0700, Andrii Nakryiko wrote:
> > On Sun, Apr 3, 2022 at 6:14 PM Andrii Nakryiko
> > <andrii.nakryiko@...il.com> wrote:
> > > 
> > > On Wed, Mar 30, 2022 at 8:27 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/binary:[raw_offset|[function_name[+offset]]")
> > > > 
> > > > 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 | 74
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++--
> > > >  1 file changed, 72 insertions(+), 2 deletions(-)
> > > > 
> > > 
> > > [...]
> > > 
> > > > +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;
> > > > +
> > > > +       opts.retprobe = str_has_pfx(prog->sec_name,
> > > > "uretprobe/");
> > > > +       if (opts.retprobe)
> > > > +               probe_name = prog->sec_name +
> > > > sizeof("uretprobe/") - 1;
> > > > +       else
> > > > +               probe_name = prog->sec_name + sizeof("uprobe/") -
> > > > 1;
> > > 
> > > I think this will mishandle SEC("uretprobe"), let's fix this in a
> > > follow up (and see a note about uretprobe selftests)
> > 
> > So I actually fixed it up a little bit to avoid test failure on s390x
> > arch. But now it's a different problem, complaining about not being

Thanks for doing all the fix-ups Andrii, and to Ilya for the Debian/s390 
and selftests fixups!

> > able to resolve libc.so.6. CC'ing Ilya, but I was wondering if it's
> > better to use more generic "libc.so" instead of "libc.so.6"? Have you
> > tried that?
> 

I looked at that, and unfortunately it's tricky because on some platforms
libc.so is a text GNU ld config file - here's what it looks like on my 
system:

$ cat /usr/lib64/libc.so
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library, so try that secondarily.  */
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( /lib64/libc.so.6 /usr/lib64/libc_nonshared.a  AS_NEEDED ( 
/lib64/ld-linux-x86-64.so.2 ) )

I tried the dlopen()/dlinfo() trick with libc.so, thinking we might be 
able to tap into native linking mechanisms such that it would parse 
that file, but it doesn't work for dlopen()ing libc.so unfortunately; 
it needed the .6 suffix.
 
> I believe it's a Debian-specific issue (our s390x CI image is Debian).
> libc is still called libc.so.6, but it's located in
> /lib/s390x-linux-gnu.
> This must also be an issue on Intel and other architectures.
> I'll send a patch.
> 

Thanks!