1. Forum
  2. Usenet
  3. LINUX.DEBIAN.PORTS.ALPHA

  • [PATCH] elf: Fix GL(dl_phdr) and GL(dl_phnum) for static builds [BZ #29

    From Adhemerval Zanella@21:1/5 to All on Tue Jan 3 15:50:02 2023
    The 73fc4e28b9464f0e refactor did not add the GL(dl_phdr) and
    GL(dl_phnum) for static build, relying on the __ehdr_start symbol,
    which is always added by the static linker, to get the correct values.

    This is problematic in some ways:

    - The segment may see its in-memory size differ from its in-file
    size (or the binary may have holes). The Linux has fixed is to
    provide concise values for both AT_PHDR and AT_PHNUM (commit
    0da1d5002745c - "fs/binfmt_elf: Fix AT_PHDR for unusual ELF files")

    - Some archs (alpha for instance) the hidden weak reference is not
    correctly pulled by the static linker and __ehdr_start address
    end up being 0, which makes GL(dl_phdr) and GL(dl_phnum) have both
    invalid values (and triggering a segfault later on libc.so while
    accessing TLS variables).

    The safer fix is to just restore the previous behavior to setup
    GL(dl_phdr) and GL(dl_phnum) for static based on kernel auxv. The
    __ehdr_start fallback can also be simplified by not assuming weak
    linkage (as for PIE).

    The libc-static.c auxv init logic is moved to dl-support.c, since
    the later is build without SHARED and then GLRO macro is defined
    to access the variables directly.

    The _dl_phdr is also assumed to be always non NULL, since an invalid
    NULL values does not trigger TLS initialization (which is used in
    various libc systems).

    Checked on aarch64-linux-gnu, x86_64-linux-gnu, and i686-linux-gnu.
    ---
    csu/libc-start.c | 21 ----------
    csu/libc-tls.c | 25 ++++++------
    elf/dl-support.c | 52 ++++++++++++++++---------
    sysdeps/unix/sysv/linux/dl-parse_auxv.h | 1 +
    4 files changed, 46 insertions(+), 53 deletions(-)

    diff --git a/csu/libc-start.c b/csu/libc-start.c
    index 543560f36c..bfeee6d851 100644
    --- a/csu/libc-start.c
    +++ b/csu/libc-start.c
    @@ -262,28 +262,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
    }
    # endif
    _dl_aux_init (auxvec);
    - if (GL(dl_phdr) == NULL)
    # endif
    - {
    - /* Starting from binutils-2.23, the linker will define the
    - magic symbol __ehdr_start to point to our own ELF header
    - if it is visible in a segment that also includes the phdrs.
    - So we can set up _dl_phdr and _dl_phnum even without any
    - information from auxv. */
    -
    - extern const ElfW(Ehdr) __ehdr_start
    -# if BUILD_PIE_DEFAULT
    - __attribute__ ((vi
  • From Florian Weimer@21:1/5 to All on Wed Jan 11 15:50:01 2023
    * Adhemerval Zanella:

    diff --git a/elf/dl-support.c b/elf/dl-support.c
    index 614b5b3e0c..b5ec5bd6d1 100644
    --- a/elf/dl-support.c
    +++ b/elf/dl-support.c
    @@ -250,12 +250,27 @@ _dl_aux_init (ElfW(auxv_t) *av)
    #endif

    _dl_auxv = av;
    - dl_parse_auxv_t auxv_values;
    - /* Use an explicit initialization loop here because memset may not
    - be available yet. */
    - for (int i = 0; i < array_length (auxv_values); ++i)
    - auxv_values[i] = 0;
    + dl_parse_auxv_t auxv_values = { 0, };

    Is this really safe? If so, it should still be in a separate commit.

    diff --git a/sysdeps/unix/sysv/linux/dl-parse_auxv.h b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
    index bf9374371e..2bf3a0ca6b 100644
    --- a/sysdeps/unix/sysv/linux/dl-parse_auxv.h
    +++ b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
    @@ -21,6 +21,7 @@
    #include <fpu_control.h>
    #include <ldsodefs.h>
    #include <link.h>
    +#include <dl-auxv.h> /* For DL_PLATFORM_AUXV */

    typedef ElfW(Addr) dl_parse_auxv_t[AT_MINSIGSTKSZ + 1];

    Why is this change needed?

    Thanks,
    Florian

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Adhemerval Zanella Netto@21:1/5 to Florian Weimer on Wed Jan 11 19:10:02 2023
    On 11/01/23 11:27, Florian Weimer wrote:
    * Adhemerval Zanella:

    diff --git a/elf/dl-support.c b/elf/dl-support.c
    index 614b5b3e0c..b5ec5bd6d1 100644
    --- a/elf/dl-support.c
    +++ b/elf/dl-support.c
    @@ -250,12 +250,27 @@ _dl_aux_init (ElfW(auxv_t) *av)
    #endif

    _dl_auxv = av;
    - dl_parse_auxv_t auxv_values;
    - /* Use an explicit initialization loop here because memset may not
    - be available yet. */
    - for (int i = 0; i < array_length (auxv_values); ++i)
    - auxv_values[i] = 0;
    + dl_parse_auxv_t auxv_values = { 0, };

    Is this really safe? If so, it should still be in a separate commit.

    It should be since 5355f9ca7b10183ce06e8a18003ba30f43774858, but I will recheck. I can move it to another patch, but I think it is a lefover of
    the -fno-tree-loop-distribute-patterns removal.


    diff --git a/sysdeps/unix/sysv/linux/dl-parse_auxv.h b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
    index bf9374371e..2bf3a0ca6b 100644
    --- a/sysdeps/unix/sysv/linux/dl-parse_auxv.h
    +++ b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
    @@ -21,6 +21,7 @@
    #include <fpu_control.h>
    #include <ldsodefs.h>
    #include <link.h>
    +#include <dl-auxv.h> /* For DL_PLATFORM_AUXV */

    typedef ElfW(Addr) dl_parse_auxv_t[AT_MINSIGSTKSZ + 1];

    Why is this change needed?

    Hum, it might a leftover of development. I will check.

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)