Re: [RFC] .BTF section data alignment issue on s390

[Jiri Olsa <jolsa@redhat.com>]

On Fri, Jun 12, 2020 at 12:46:13AM +0200, Ilya Leoshkevich wrote:
> On Thu, 2020-06-11 at 22:50 +0200, Jiri Olsa wrote:
> > hi,
> > we're hitting a problem on s390 with BTF data alignment.
> > 
> > When running simple test, we're getting this message from
> > verifier and console:
> > 
> >   bpf_common.c:91: BROK: Failed verification: in-kernel BTF is
> > malformed
> >   [   41.545572] BPF:Total section length too long
> > 
> > 
> > AFAICS it happens when .BTF section data size is not an even number
> > ;-)
> > 
> > DISCLAIMER I'm quite ignorant of s390x arch details, so most likely
> > I'm
> > totally wrong and perhaps missing something important and there's
> > simple
> > explanation.. but here's what got me here:
> > 
> > 
> > ... so BTF data is placed in .BTF section via linker script:
> > 
> >         .BTF : AT(ADDR(.BTF) - LOAD_OFFSET)
> > {                           \
> >                 __start_BTF =
> > .;                                        \
> >                 *(.BTF)                                              
> >    \
> >                 __stop_BTF =
> > .;                                         \
> >         }
> > 
> > 
> > and the .BTF data size in btf_parse_vmlinux is computed as:
> > 
> >         btf->data_size = __stop_BTF - __start_BTF;
> > 
> > 
> > this computation is compiled as:
> > 
> >         00000000002aeb20 <btf_parse_vmlinux>:
> >         ...
> >           2aeb8a:  larl    %r1,cda3ac <__start_BTF+0x2084a8>    #
> > loads r1 with end
> >           2aeb90:  larl    %r2,ad1f04 <__start_BTF>             #
> > loads r2 with start
> >           2aeb96:  sgr     %r1,%r2                              #
> > substract r1 - r2 
> > 
> > 
> > having following values for start/stop_BTF symbols:
> > 
> >         # nm ./vmlinux | grep __start_BTF
> >         0000000000ad1f04 R __start_BTF
> >         # nm ./vmlinux | grep __stop_BTF
> >         0000000000cda3ad R __stop_BTF
> > 
> >         -> the BTF data size is 0x2084a9
> > 
> > 
> > but as you can see the instruction that loads the 'end' symbol:
> > 
> >         larl    %r1,cda3ac <__start_BTF+0x2084a8>
> > 
> > 
> > is loading '__start_BTF + 0x2084a8', which is '__stop_BTF - 1'
> > 
> > 
> > From spec it seems that larl instruction's argument must be even
> > number ([1] page 7-214):
> > 
> >         2.   For  LOAD  RELATIVE  LONG,  the  second  oper-and must
> > be aligned
> >         on an integral boundary cor-responding to the operand’s
> > size. 
> > 
> > 
> > I also found an older bug complaining about this issue [2]:
> > 
> >         ...
> >         larl instruction can only load even values - instructions on
> > s390 are 2-byte
> >         aligned and the instruction encodes offset to the target in
> > 2-byte units.
> >         ...
> >         The GNU BFD linker for s390 doesn't bother to check if
> > relocations fit or are
> >         properly aligned. 
> >         ...
> > 
> > 
> > I tried to fix that aligning the end to even number, but then
> > btf_check_sec_info logic needs to be adjusted as well, and
> > probably other places as well.. so I decided to share this
> > first.. because it all seems wrong ;-)
> > 
> > thoughts? thanks,
> > jirka
> > 
> > 
> > [1] http://publibfi.boulder.ibm.com/epubs/pdf/dz9zr008.pdf
> > [2] https://sourceware.org/bugzilla/show_bug.cgi?id=18960
> > 
> Hi Jiri,
> 
> Actually I recently ran into it myself on Debian, and I believe your
> analysis is correct :-) The only thing to add to it is that the
> compiler emits the correct instruction (if you look at the .o file),
> it's linker that messes things up.
> 
> The linker bug in question is [1].
> 
> I opened [2] to Debian folks, and I believe that other important
> distros (RH, SUSE, Ubuntu) have this fixed already.
> 
> Which distro are you using?

I'm on RHEL ;-) I wonder why that fix was missed,
I'll follow up on that with our binutils guys

thanks a lot for the info,
jirka

> 
> Best regards,
> Ilya
> 
> [1] 
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=e6213e09ed0e
> [2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=961736
>