Bug 26312 - ld produces broken PLT on aarch64 with BTI+PAC
Summary: ld produces broken PLT on aarch64 with BTI+PAC
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.35
: P2 normal
Target Milestone: 2.36
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-29 12:31 UTC by Florian Weimer
Modified: 2020-08-11 11:12 UTC (History)
5 users (show)

See Also:
Host:
Target: aarch64
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Weimer 2020-07-29 12:31:09 UTC
Building glibc 2.32 on Fedora rawhide with GCC 10.2, -mbranch-protection=standard, and binutils 2.35 results in a libc.so.6 which lacks PAC support, possibly due to missing PAC in libgcc.a for the outline atomics. (We build with -moutline-atomics as well.) This in itself should not be a problem.

However, catgets/gencat is mislinked.  The PLT is corrupted because its entry size is not constant (32 bytes for the first entry, 24 bytes for subsequent entryes, section table says 24 bytes):

Disassembly of section .plt:

0000000000401140 <.plt>:
  401140:       d503245f        bti     c
  401144:       a9bf7bf0        stp     x16, x30, [sp, #-16]!
  401148:       d00000f0        adrp    x16, 41f000 <__FRAME_END__+0x1abd4>
  40114c:       f9474a11        ldr     x17, [x16, #3728]
  401150:       913a4210        add     x16, x16, #0xe90
  401154:       d61f0220        br      x17
  401158:       d503201f        nop
  40115c:       d503201f        nop

0000000000401160 <memcpy@plt>:
  401160:       d503245f        bti     c
  401164:       d00000f0        adrp    x16, 41f000 <__FRAME_END__+0x1abd4>
  401168:       f9474e11        ldr     x17, [x16, #3736]
  40116c:       913a6210        add     x16, x16, #0xe98
  401170:       d61f0220        br      x17
  401174:       d503201f        nop

0000000000401178 <strlen@plt>:
  401178:       d503245f        bti     c
  40117c:       d00000f0        adrp    x16, 41f000 <__FRAME_END__+0x1abd4>
  401180:       f9475211        ldr     x17, [x16, #3744]
  401184:       913a8210        add     x16, x16, #0xea0
  401188:       d61f0220        br      x17
  40118c:       d503201f        nop

I mentioned the lack of PAC earlier because ld seems to be confused about the PAC status.  It only sets DT_AARCH64_BTI_PLT:

Dynamic section at offset 0xfc60 contains 29 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-aarch64.so.1]
 0x000000000000000c (INIT)               0x401120
 0x000000000000000d (FINI)               0x403868
 0x0000000000000019 (INIT_ARRAY)         0x41fc40
 0x000000000000001b (INIT_ARRAYSZ)       8 (bytes)
 0x000000000000001a (FINI_ARRAY)         0x41fc48
 0x000000000000001c (FINI_ARRAYSZ)       8 (bytes)
 0x0000000000000004 (HASH)               0x400330
 0x000000006ffffef5 (GNU_HASH)           0x400498
 0x0000000000000005 (STRTAB)             0x400990
 0x0000000000000006 (SYMTAB)             0x4004e0
 0x000000000000000a (STRSZ)              575 (bytes)
 0x000000000000000b (SYMENT)             24 (bytes)
 0x0000000000000015 (DEBUG)              0x0
 0x0000000000000003 (PLTGOT)             0x41fe80
 0x0000000000000002 (PLTRELSZ)           1008 (bytes)
 0x0000000000000014 (PLTREL)             RELA
 0x0000000000000017 (JMPREL)             0x400d30
 0x0000000000000007 (RELA)               0x400c88
 0x0000000000000008 (RELASZ)             168 (bytes)
 0x0000000000000009 (RELAENT)            24 (bytes)
 0x0000000070000001 (AARCH64_BTI_PLT)    
 0x0000000000000018 (BIND_NOW)           
 0x000000006ffffffb (FLAGS_1)            Flags: NOW
 0x000000006ffffffe (VERNEED)            0x400c38
 0x000000006fffffff (VERNEEDNUM)         2
 0x000000006ffffff0 (VERSYM)             0x400bd0
 0x0000000000000000 (NULL)               0x0

But the note says it has both:

Displaying notes found in: .note.gnu.property
  Owner                Data size        Description
  GNU                  0x00000010       NT_GNU_PROPERTY_TYPE_0
      Properties: AArch64 feature: BTI, PAC
Comment 1 Jakub Jelinek 2020-07-29 13:01:14 UTC
Non-uniform .plt sizes aren't necessarily a problem, but in that case sh_entsize needs to be changed, either to 0, 1, 4 or perhaps 8, but from the description 0 is probably the best choice.
E.g. powerpc 32-bit is using sh_entsize of 0 for .plt too.
Comment 2 Szabolcs Nagy 2020-07-29 13:33:03 UTC
the lack of PAC in PLT is deliberate: building with pac-ret does not
enable pac-plt and glibc has no support for pac-plt anyway (ld.so
would have to store signed addresses in PLTGOT entries)

the PAC note is not very useful/obvious: it's documented to mark
objects where pac-ret was in use throughout the code. (the dynamic
linker does not have to do anything with it, it just allows tracking
when something is not PAC protected).

normal plt entry size is 16bytes, but the first plt entry size is
always 32bytes.

with DT_AARCH64_BTI_PLT the plt entry size is 24bytes, and the first
plt entry size is 32bytes.

i don't know about sh_entsize, i will have to check what it should be.
Comment 3 Mark Wielaard 2020-07-29 13:44:49 UTC
(In reply to Szabolcs Nagy from comment #2)
> i don't know about sh_entsize, i will have to check what it should be.

In general sh_size modulo sh_entsize needs to be zero (if sh_entsize isn't zero itself). For non-zero sh_entsize sections, elfutils libelf will flag a section as  corrupt if the sh_size % sh_entsize != 0.

In this case, given that you have entries of variable size, I believe sh_entsize should be zero.
Comment 4 Jakub Jelinek 2020-07-29 13:53:46 UTC
The gABI says:
sh_entsize
    Some sections hold a table of fixed-size entries, such as a symbol table. For such a section, this member gives the size in bytes of each entry. The member contains 0 if the section does not hold a table of fixed-size entries.
Comment 5 Florian Weimer 2020-07-29 13:59:10 UTC
There are also PLT patching tools which would benefit from accurate entry sizes.  (ltrace is an example, I believe.)
Comment 6 Szabolcs Nagy 2020-07-30 09:08:27 UTC
note on 32bit arm, sh and or1k .plt sh_entsize == 4 which
has nothing to do whith the plt entry size (which can
vary on arm). on sparc the logic is more complicated and
the plt entry size is used in some cases but the plt
header is bigger (it's a multiple of the entry size though).
and there are targets where it is 0.

to me this tells that sh_entsize of .plt is not reliable
and should be just ignored by tools (ltrace cannot use
it either).

i will set it to 0 in all cases on aarch64 and hope
nothing will break.

(note that the bug only affects executables, not shared
libs: only executables use special plt for bti because
then the plt can provide the canonical address of function
symbols, in libraries the plt is not called indirectly)
Comment 8 Sourceware Commits 2020-07-30 16:02:17 UTC
The master branch has been updated by Szabolcs Nagy <nsz@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4d3bb35620e70d543d438bf21be1307f7ea0f5d0

commit 4d3bb35620e70d543d438bf21be1307f7ea0f5d0
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Wed Jul 29 15:47:50 2020 +0100

    aarch64: set sh_entsize of .plt to 0
    
    On aarch64 the first PLT entry is 32 bytes, subsequent entries
    are 16 bytes by default but can be 24 bytes with BTI or with
    PAC-PLT.
    
    sh_entsize of .plt was set to the PLT entry size, so in some
    cases sh_size % sh_entsize != 0, which breaks some tools.
    
    Note that PLT0 (and the TLSDESC stub code which is also in the
    PLT) were historically not padded up to meet the sh_size
    requirement, but to ensure that PLT stub code is aligned on
    cache lines. Similar layout is present on other targets too
    which just happens to make sh_size a multiple of sh_entsize and
    it is not expected that sh_entsize of .plt is used for anything.
    
    This patch sets sh_entsize of .plt to 0: the section does not
    hold a table of fixed-size entries so other values are not
    conforming in principle to the ELF spec.
    
    bfd/ChangeLog:
    
            PR ld/26312
            * elfnn-aarch64.c (elfNN_aarch64_init_small_plt0_entry): Set sh_entsize
            to 0.
            (elfNN_aarch64_finish_dynamic_sections): Remove sh_entsize setting.
Comment 9 Sourceware Commits 2020-07-30 16:42:17 UTC
The binutils-2_35-branch branch has been updated by Szabolcs Nagy <nsz@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4d4f8ee98118230510f20c3ff27b66e0132020dc

commit 4d4f8ee98118230510f20c3ff27b66e0132020dc
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Wed Jul 29 15:47:50 2020 +0100

    aarch64: set sh_entsize of .plt to 0
    
    On aarch64 the first PLT entry is 32 bytes, subsequent entries
    are 16 bytes by default but can be 24 bytes with BTI or with
    PAC-PLT.
    
    sh_entsize of .plt was set to the PLT entry size, so in some
    cases sh_size % sh_entsize != 0, which breaks some tools.
    
    Note that PLT0 (and the TLSDESC stub code which is also in the
    PLT) were historically not padded up to meet the sh_size
    requirement, but to ensure that PLT stub code is aligned on
    cache lines. Similar layout is present on other targets too
    which just happens to make sh_size a multiple of sh_entsize and
    it is not expected that sh_entsize of .plt is used for anything.
    
    This patch sets sh_entsize of .plt to 0: the section does not
    hold a table of fixed-size entries so other values are not
    conforming in principle to the ELF spec.
    
    bfd/ChangeLog:
    
            PR ld/26312
            * elfnn-aarch64.c (elfNN_aarch64_init_small_plt0_entry): Set sh_entsize
            to 0.
            (elfNN_aarch64_finish_dynamic_sections): Remove sh_entsize setting.
    
    (cherry picked from commit 4d3bb35620e70d543d438bf21be1307f7ea0f5d0)
Comment 10 Sourceware Commits 2020-08-11 11:10:32 UTC
The binutils-2_34-branch branch has been updated by Szabolcs Nagy <nsz@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e3b314d3a61db4b0b36975fa00eb9043a6142448

commit e3b314d3a61db4b0b36975fa00eb9043a6142448
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Wed Jul 29 15:47:50 2020 +0100

    aarch64: set sh_entsize of .plt to 0
    
    On aarch64 the first PLT entry is 32 bytes, subsequent entries
    are 16 bytes by default but can be 24 bytes with BTI or with
    PAC-PLT.
    
    sh_entsize of .plt was set to the PLT entry size, so in some
    cases sh_size % sh_entsize != 0, which breaks some tools.
    
    Note that PLT0 (and the TLSDESC stub code which is also in the
    PLT) were historically not padded up to meet the sh_size
    requirement, but to ensure that PLT stub code is aligned on
    cache lines. Similar layout is present on other targets too
    which just happens to make sh_size a multiple of sh_entsize and
    it is not expected that sh_entsize of .plt is used for anything.
    
    This patch sets sh_entsize of .plt to 0: the section does not
    hold a table of fixed-size entries so other values are not
    conforming in principle to the ELF spec.
    
    bfd/ChangeLog:
    
            Backport from mainline.
            2020-07-30  Szabolcs Nagy  <szabolcs.nagy@arm.com>
    
            PR ld/26312
            * elfnn-aarch64.c (elfNN_aarch64_init_small_plt0_entry): Set sh_entsize
            to 0.
            (elfNN_aarch64_finish_dynamic_sections): Remove sh_entsize setting.
    
    (cherry picked from commit 4d3bb35620e70d543d438bf21be1307f7ea0f5d0)
Comment 11 Szabolcs Nagy 2020-08-11 11:12:13 UTC
fixed for 2.36 and on the 2.35 and 2.34 branches.