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

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:

quoted

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