Re: [PATCH v2 3/3] btf: Avoid weak external references

[Ard Biesheuvel]

On Wed, 10 Apr 2024 at 10:22, Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Tue, Apr 09, 2024 at 05:01:36PM +0200, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@xxxxxxxxxx>
> >
> > If the BTF code is enabled in the build configuration, the start/stop
> > BTF markers are guaranteed to exist in the final link but not during the
> > first linker pass.
> >
> > Avoid GOT based relocations to these markers in the final executable by
> > providing preliminary definitions that will be used by the first linker
> > pass, and superseded by the actual definitions in the subsequent ones.
> >
> > Make the preliminary definitions dependent on CONFIG_DEBUG_INFO_BTF so
> > that inadvertent references to this section will trigger a link failure
> > if they occur in code that does not honour CONFIG_DEBUG_INFO_BTF.
> >
> > Note that Clang will notice that taking the address of__start_BTF cannot
> > yield NULL any longer, so testing for that condition is no longer
> > needed.
> >
> > Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > ---
> >  include/asm-generic/vmlinux.lds.h | 9 +++++++++
> >  kernel/bpf/btf.c                  | 4 ++--
> >  kernel/bpf/sysfs_btf.c            | 6 +++---
> >  3 files changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index e8449be62058..4cb3d88449e5 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -456,6 +456,7 @@
> >   * independent code.
> >   */
> >  #define PRELIMINARY_SYMBOL_DEFINITIONS                                       \
> > +     PRELIMINARY_BTF_DEFINITIONS                                     \
> >       PROVIDE(kallsyms_addresses = .);                                \
> >       PROVIDE(kallsyms_offsets = .);                                  \
> >       PROVIDE(kallsyms_names = .);                                    \
> > @@ -466,6 +467,14 @@
> >       PROVIDE(kallsyms_markers = .);                                  \
> >       PROVIDE(kallsyms_seqs_of_names = .);
> >
> > +#ifdef CONFIG_DEBUG_INFO_BTF
> > +#define PRELIMINARY_BTF_DEFINITIONS                                  \
> > +     PROVIDE(__start_BTF = .);                                       \
> > +     PROVIDE(__stop_BTF = .);
> > +#else
> > +#define PRELIMINARY_BTF_DEFINITIONS
> > +#endif
>
> hi,
> I'm getting following compilation fail when CONFIG_DEBUG_INFO_BTF is disabled
>
>         [jolsa@krava linux-qemu]$ make
>           CALL    scripts/checksyscalls.sh
>           DESCEND objtool
>           INSTALL libsubcmd_headers
>           UPD     include/generated/utsversion.h
>           CC      init/version-timestamp.o
>           LD      .tmp_vmlinux.kallsyms1
>         ld: kernel/bpf/btf.o: in function `btf_parse_vmlinux':
>         /home/jolsa/kernel/linux-qemu/kernel/bpf/btf.c:5988: undefined reference to `__start_BTF'
>         ld: /home/jolsa/kernel/linux-qemu/kernel/bpf/btf.c:5989: undefined reference to `__stop_BTF'
>         ld: /home/jolsa/kernel/linux-qemu/kernel/bpf/btf.c:5989: undefined reference to `__start_BTF'
>         make[2]: *** [scripts/Makefile.vmlinux:37: vmlinux] Error 1
>         make[1]: *** [/home/jolsa/kernel/linux-qemu/Makefile:1160: vmlinux] Error 2
>         make: *** [Makefile:240: __sub-make] Error 2
>
> maybe the assumption was that kernel/bpf/btf.o is compiled only
> for CONFIG_DEBUG_INFO_BTF, but it's actually:
>
>   obj-$(CONFIG_BPF_SYSCALL) += btf.o memalloc.o
>

Interesting. I did build test this with and without
CONFIG_DEBUG_INFO_BTF, but not with CONFIG_BPF_SYSCALL=y and
CONFIG_DEBUG_INFO_BTF=n.

> I guess we just need !CONFIG_DEBUG_INFO_BTF version of btf_parse_vmlinux
> function
>

The below gives me a working build.

--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5971,6 +5971,9 @@ struct btf *btf_parse_vmlinux(void)
        struct btf *btf = NULL;
        int err;

+       if (!IS_ENABLED(CONFIG_DEBUG_INFO_BTF))
+               return ERR_PTR(-ENOENT);
+
        env = kzalloc(sizeof(*env), GFP_KERNEL | __GFP_NOWARN);
        if (!env)
                return ERR_PTR(-ENOMEM);