Re: [PATCH bpf-next] libbpf: fix internal USDT address translation logic for shared libraries

Andrii Nakryiko
2022-06-16T20:39:02Z

On Thu, Jun 16, 2022 at 1:00 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Jun 15, 2022 at 10:56 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> >
> > Perform the same virtual address to file offset translation that libbpf
> > is doing for executable ELF binaries also for shared libraries.
> > Currently libbpf is making a simplifying and sometimes wrong assumption
> > that for shared libraries relative virtual addresses inside ELF are
> > always equal to file offsets.
> >
> > Unfortunately, this is not always the case with LLVM's lld linker, which
> > now by default generates quite more complicated ELF segments layout.
> > E.g., for liburandom_read.so from selftests/bpf, here's an excerpt from
> > readelf output listing ELF segments (a.k.a. program headers):
> >
> > Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
> > PHDR 0x000040 0x0000000000000040 0x0000000000000040 0x0001f8 0x0001f8 R 0x8
> > LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x0005e4 0x0005e4 R 0x1000
> > LOAD 0x0005f0 0x00000000000015f0 0x00000000000015f0 0x000160 0x000160 R E 0x1000
> > LOAD 0x000750 0x0000000000002750 0x0000000000002750 0x000210 0x000210 RW 0x1000
> > LOAD 0x000960 0x0000000000003960 0x0000000000003960 0x000028 0x000029 RW 0x1000
> >
> > Compare that to what is generated by GNU ld (or LLVM lld's with extra
> > -znoseparate-code argument which disables this cleverness in the name of
> > file size reduction):
> >
> > Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
> > LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x000550 0x000550 R 0x1000
> > LOAD 0x001000 0x0000000000001000 0x0000000000001000 0x000131 0x000131 R E 0x1000
> > LOAD 0x002000 0x0000000000002000 0x0000000000002000 0x0000ac 0x0000ac R 0x1000
> > LOAD 0x002dc0 0x0000000000003dc0 0x0000000000003dc0 0x000262 0x000268 RW 0x1000
> >
> > You can see from the first example above that for executable (Flg == "R E")
> > PT_LOAD segment (LOAD #2), Offset doesn't match VirtAddr columns.
> > And it does in the second case (GNU ld output).
> >
> > This is important because all the addresses, including USDT specs,
> > operate in a virtual address space, while kernel is expecting file
> > offsets when performing uprobe attach. So such mismatches have to be
> > properly taken care of and compensated by libbpf, which is what this
> > patch is fixing.
> >
> > Also patch clarifies few function and variable names, as well as updates
> > comments to reflect this important distinction (virtaddr vs file offset)
> > and to ephasize that shared libraries are not all that different from
> > executables in this regard.
> >
> > This patch also changes selftests/bpf Makefile to force urand_read and
> > liburand_read.so to be built with Clang and LLVM's lld (and explicitly
> > request this ELF file size optimization through -znoseparate-code linker
> > parameter) to validate libbpf logic and ensure regressions don't happen
> > in the future. I've bundled these selftests changes together with libbpf
> > changes to keep the above description tied with both libbpf and
> > selftests changes.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > tools/lib/bpf/usdt.c | 123 ++++++++++++++-------------
>
> What should be the Fixes tag here?
> Back to the beginning of this usdt.c file?

Oh, sorry, forgot about the Fixes tag. Yeah, it's pretty much never
worked properly for such fancy ELF shared libraries.

Fixes: 74cc6311cec9 ("libbpf: Add USDT notes parsing and resolution logic")