Bug 28689 - p_align in ELF program headers should not exceed section alignment
Summary: p_align in ELF program headers should not exceed section alignment
Status: NEW
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.38
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-13 15:00 UTC by Florian Weimer
Modified: 2022-01-05 13:25 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
A patch (1.90 KB, patch)
2021-12-14 03:51 UTC, H.J. Lu
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Weimer 2021-12-13 15:00:48 UTC
Currently, on 32-bit and 64-bit Arm, it seems that BFD ld generates p_align values of 65536 even if no section alignment is greater than 4096. The issue is more general and probably affects other targets with multiple common page sizes.

While file layout absolutely must take 64K page size into account, that does not have to be reflected in the p_align value. If running on a 64K kernel, the file will be loaded at a 64K page boundary by necessity. On a 4K kernel, 64K alignment is not needed.

We are going to fix the glibc loader to honor p_align (bug 28676). This means that on 4K kernels, we will start to do extra work for 64K p_align, but this pointless for pretty much all binaries (whose section alignment rarely exceeds 16).

A further complication is glibc bug 28688. So I'm not entirely sure anymore what we should do here.
Comment 1 H.J. Lu 2021-12-13 21:48:34 UTC
I think p_align should be set to ELF_MINPAGESIZE by default.
Comment 2 H.J. Lu 2021-12-13 23:53:39 UTC
One problem:

Elf file type is EXEC (Executable file)
Entry point 0x600000
There are 7 program headers, starting at offset 64

Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x0001f8 0x0001f8 R   0x1000
  LOAD           0x200000 0x0000000000600000 0x0000000000600000 0x000001 0x000001 R E 0x1000
  LOAD           0x400000 0x0000000000800000 0x0000000000800000 0x002000 0x002000 RW  0x1000
  NOTE           0x0001c8 0x00000000004001c8 0x00000000004001c8 0x000030 0x000030 R   0x8
  TLS            0x400000 0x0000000000800000 0x0000000000800000 0x001000 0x001000 R   0x10
  GNU_PROPERTY   0x0001c8 0x00000000004001c8 0x00000000004001c8 0x000030 0x000030 R   0x8
  GNU_RELRO      0x400000 0x0000000000800000 0x0000000000800000 0x001000 0x001000 R   0x1

But objcopy will take p_align as the maximum page size:

Elf file type is EXEC (Executable file)
Entry point 0x600000
There are 7 program headers, starting at offset 64

Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x0001f8 0x0001f8 R   0x1000
  LOAD           0x001000 0x0000000000600000 0x0000000000600000 0x000001 0x000001 R E 0x1000
  LOAD           0x002000 0x0000000000800000 0x0000000000800000 0x002000 0x002000 RW  0x1000
  NOTE           0x0001c8 0x00000000004001c8 0x00000000004001c8 0x000030 0x000030 R   0x8
  TLS            0x002000 0x0000000000800000 0x0000000000800000 0x001000 0x001000 R   0x10
  GNU_PROPERTY   0x0001c8 0x00000000004001c8 0x00000000004001c8 0x000030 0x000030 R   0x8
  GNU_RELRO      0x002000 0x0000000000800000 0x0000000000800000 0x001000 0x001000 R   0x1

File offsets are changed.
Comment 3 H.J. Lu 2021-12-14 03:51:30 UTC
Created attachment 13850 [details]
A patch

Does this work?
Comment 4 Fangrui Song 2021-12-14 05:24:00 UTC
(In reply to H.J. Lu from comment #2)
> One problem:
> [...]
> File offsets are changed.

For objcopy, it is important to not transform the file to decrease the intrinsic max-page-size. This would make the output not runnable on the originally supported systems.

If ld decreases p_align to reduce wasted memory pages, can you describe how the objcopy problem (inferring max-page-size) will be solved?
Note that max-page-size can be trick to infer in the -z noseparate-code case.
Comment 5 Fangrui Song 2021-12-14 05:47:13 UTC
(In reply to Fangrui Song from comment #4)
> (In reply to H.J. Lu from comment #2)
> > One problem:
> > [...]
> > File offsets are changed.
> 
> For objcopy, it is important to not transform the file to decrease the
> intrinsic max-page-size. This would make the output not runnable on the
> originally supported systems.
> 
> If ld decreases p_align to reduce wasted memory pages, can you describe how
> the objcopy problem (inferring max-page-size) will be solved?
> Note that max-page-size can be trick to infer in the -z noseparate-code case.

Maybe something like gcd({p_vaddr - p_offset}). If the gcd is 0, pick p_align
Comment 6 H.J. Lu 2021-12-14 14:14:14 UTC
(In reply to Fangrui Song from comment #5)
> (In reply to Fangrui Song from comment #4)
> > (In reply to H.J. Lu from comment #2)
> > > One problem:
> > > [...]
> > > File offsets are changed.
> > 
> > For objcopy, it is important to not transform the file to decrease the
> > intrinsic max-page-size. This would make the output not runnable on the
> > originally supported systems.
> > 
> > If ld decreases p_align to reduce wasted memory pages, can you describe how
> > the objcopy problem (inferring max-page-size) will be solved?
> > Note that max-page-size can be trick to infer in the -z noseparate-code case.
> 
> Maybe something like gcd({p_vaddr - p_offset}). If the gcd is 0, pick p_align

This is roughly what my patch does.
Comment 7 cvs-commit@gcc.gnu.org 2022-01-05 13:11:30 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

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

commit 74e315dbfe5200c473b226e937935fb8ce391489
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Dec 13 19:46:04 2021 -0800

    elf: Set p_align to the minimum page size if possible
    
    Currently, on 32-bit and 64-bit ARM, it seems that ld generates p_align
    values of 0x10000 even if no section alignment is greater than 0x1000.
    The issue is more general and probably affects other targets with multiple
    page sizes.
    
    While file layout absolutely must take 64K page size into account, that
    does not have to be reflected in the p_align value.  If running on a 64K
    kernel, the file will be loaded at a 64K page boundary by necessity. On
    a 4K kernel, 64K alignment is not needed.
    
    The glibc loader has been fixed to honor p_align:
    
    https://sourceware.org/bugzilla/show_bug.cgi?id=28676
    
    similar to kernel:
    
    commit ce81bb256a224259ab686742a6284930cbe4f1fa
    Author: Chris Kennelly <ckennelly@google.com>
    Date:   Thu Oct 15 20:12:32 2020 -0700
    
        fs/binfmt_elf: use PT_LOAD p_align values for suitable start address
    
    This means that on 4K kernels, we will start to do extra work for 64K
    p_align, but this pointless for pretty much all binaries (whose section
    alignment rarely exceeds 16).
    
    The minimum page size is used, instead of the maximum section alignment
    due to this glibc bug:
    
    https://sourceware.org/bugzilla/show_bug.cgi?id=28688
    
    It has been fixed in glibc 2.35.  But linker output must work on existing
    glibc binaries.
    
    1. Set p_align to the minimum page size while laying out segments aligning
    to the maximum page size or section alignment.  The run-time loader can
    align segments to the minimum page size or above, depending on system page
    size.
    2. If -z max-page-size=NNN is used, p_align will be set to the maximum
    page size or the largest section alignment.
    3. If a section requires alignment higher than the minimum page size,
    don't set p_align to the minimum page size.
    4. If a section requires alignment higher than the maximum page size,
    set p_align to the section alignment.
    5. For objcopy, when the minimum page size != the maximum page size,
    p_align may be set to the minimum page size while segments are aligned
    to the maximum page size.  In this case, the input p_align will be
    ignored and the maximum page size will be used to align the ouput
    segments.
    6. Update linker to disallow the common page size > the maximum page size.
    7. Update linker to avoid the common page size > the maximum page size.
    8. Adjust pru_irq_map-1.d to expect p_align == sh_addralign:
    
    Section Headers:
      [Nr] Name   Type            Addr     Off    Size   ES Flg Lk Inf Al
      [ 0]        NULL            00000000 000000 000000 00      0   0  0
      [ 1] .text  PROGBITS        20000000 00007c 000004 00  AX  0   0  4
    ...
    Program Headers:
      Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
      LOAD           0x000074 0x00000000 0x00000000 0x00008 0x00008 RW  0x1
      LOAD           0x00007c 0x20000000 0x20000000 0x00004 0x00004 R E 0x4
    
    vs.
    
    Section Headers:
      [Nr] Name   Type            Addr     Off    Size   ES Flg Lk Inf Al
      [ 0]        NULL            00000000 000000 000000 00      0   0  0
      [ 1] .text  PROGBITS        20000000 00007c 000004 00  AX  0   0  4
    ...
    Program Headers:
      Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
      LOAD           0x000074 0x00000000 0x00000000 0x00008 0x00008 RW  0x1
      LOAD           0x00007c 0x20000000 0x20000000 0x00004 0x00004 R E 0x1
    
    To enable this linker optimization, the backend should define ELF_P_ALIGN
    to ELF_MINPAGESIZE.
    
    bfd/
    
            PR ld/28689
            PR ld/28695
            * elf-bfd.h (elf_backend_data): Add p_align.
            * elf.c (assign_file_positions_for_load_sections): Set p_align
            to the default p_align value while laying out segments aligning
            to maximum page size or section alignment.
            (elf_is_p_align_valid): New function.
            (copy_elf_program_header): Call elf_is_p_align_valid to determine
            if p_align is valid.
            * elfxx-target.h (ELF_P_ALIGN): New.  Default to 0.
            (elfNN_bed): Add ELF_P_ALIGN.
            * elfxx-x86.h (ELF_P_ALIGN): New.  Set to ELF_MINPAGESIZE.
    
    include/
    
            PR ld/28689
            PR ld/28695
            * bfdlink.h (bfd_link_info): Add maxpagesize_is_set.
    
    ld/
    
            PR ld/28689
            PR ld/28695
            * emultempl/elf.em (gld${EMULATION_NAME}_handle_option): Set
            link_info.maxpagesize_is_set for -z max-page-size=NNN.
            * ldelf.c (ldelf_after_parse): Disallow link_info.commonpagesize
            > link_info.maxpagesize.
            * testsuite/ld-elf/elf.exp: Pass -z max-page-size=0x4000 to
            linker to build mbind2a and mbind2b.
            * testsuite/ld-elf/header.d: Add -z common-page-size=0x100.
            * testsuite/ld-elf/linux-x86.exp: Add PR ld/28689 tests.
            * testsuite/ld-elf/p_align-1.c: New file.
            * testsuite/ld-elf/page-size-1.d: New test.
            * testsuite/ld-elf/pr26936.d: Add -z common-page-size=0x1000.
            * testsuite/ld-elf/seg.d: Likewise.
            * testsuite/ld-scripts/rgn-at5.d: Likewise.
            * testsuite/ld-pru/pru_irq_map-1.d: Append 1 to name.  Adjust
            expected PT_LOAD segment alignment.
            * testsuite/ld-pru/pru_irq_map-2.d: Append 2 to name.
            * testsuite/ld-scripts/pr23571.d: Add -z max-page-size=0x1000.
Comment 8 H.J. Lu 2022-01-05 13:25:38 UTC
Fixed for x86 in binutils 2.38.