[PATCH v3 23/29] configure: Add --with-ld-relro-load-gaps configure option and test

Adhemerval Zanella Netto adhemerval.zanella@linaro.org
Fri Nov 22 20:30:04 GMT 2024



On 29/09/24 13:36, Florian Weimer wrote:
> Related to binutils bug 28743.
> 
> The new test elf/tst-gaps-ldso verifies that ld.so does not have gaps
> if the configure check indicates that the linker does not produce such
> gaps, and the architecture supports a single page size only.
> 
> The companion test elf/tst-load-alignment checks that the LOAD
> segments produced during the build permit loading glibc on the
> largest supported page size.

I failing to see the usefulness of this new configure check, if I understand
correctly on following patches it uses as an optimization on rtld.c and
dl-find_object.c to avoid some extra work for l_contiguous. It also seems
that it used solely on cold-path (initialization).

Do we really need this configure switch?

> ---
>  INSTALL                    |  12 ++++
>  config.h.in                |   3 +
>  configure                  |  39 +++++++++++
>  configure.ac               |  20 ++++++
>  elf/Makefile               |  25 +++++++
>  elf/tst-program-headers.py | 137 +++++++++++++++++++++++++++++++++++++
>  manual/install.texi        |  12 ++++
>  7 files changed, 248 insertions(+)
>  create mode 100644 elf/tst-program-headers.py
> 
> diff --git a/INSTALL b/INSTALL
> index 24e3c8d25b..91a1e5e984 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -90,6 +90,18 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
>       library will still be usable, but functionality may be lost--for
>       example, you can't build a shared libc with old binutils.
>  
> +'--with-ld-relro-load-gaps=CHOICE'
> +     If CHOICE is 'yes', assume that the linker may produce gaps between
> +     LOAD segments when linking 'ld.so', related to RELRO processing.
> +     With 'no', assume that no such gaps are produced.  The default for
> +     CHOICE is 'check', which performs a version-only linker check.  The
> +     binutils linker bug in question is
> +     <https://sourceware.org/bugzilla/show_bug.cgi?id=28743>.
> +
> +     There is another source of LOAD segment gaps on architectures which
> +     support multiple page sizes.  The '--with-ld-relro-load-gaps' is
> +     not related to that.
> +
>  '--with-nonshared-cflags=CFLAGS'
>       Use additional compiler flags CFLAGS to build the parts of the
>       library which are always statically linked into applications and
> diff --git a/config.h.in b/config.h.in
> index 6c25c923fa..00c0d391d4 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -263,6 +263,9 @@
>     any external dependencies such as making a function call.  */
>  #define HAVE_BUILTIN_TRAP 0
>  
> +/* Define if ld may produce gaps in load segments, related to RELRO.  */
> +#undef HAVE_LD_RELRO_LOAD_GAPS
> +
>  /* ports/sysdeps/mips/configure.in  */
>  /* Define if using the IEEE 754-2008 NaN encoding on the MIPS target.  */
>  #undef HAVE_MIPS_NAN2008
> diff --git a/configure b/configure
> index ec0b62db36..33f63911c6 100755
> --- a/configure
> +++ b/configure
> @@ -610,6 +610,7 @@ PACKAGE_URL='https://www.gnu.org/software/glibc/'
>  
>  ac_unique_file="include/features.h"
>  enable_option_checking=no
> +with_ld_relro_load_gaps=check
>  ac_subst_vars='LTLIBOBJS
>  LIBOBJS
>  pthread_in_libc
> @@ -809,6 +810,7 @@ enable_cet
>  enable_scv
>  enable_fortify_source
>  with_cpu
> +with_ld_relro_load_gaps
>  '
>        ac_precious_vars='build_alias
>  host_alias
> @@ -1513,6 +1515,9 @@ Optional Packages:
>    --with-man-pages=VERSION
>                            tie manual to a specific man-pages version
>    --with-cpu=CPU          select code for CPU variant
> +  --with-ld-relro-load-gaps
> +                          support linker with RELRO LOAD segment gap bug
> +                          [default=check]
>  
>  Some influential environment variables:
>    CC          C compiler command
> @@ -5319,6 +5324,40 @@ esac
>  config_vars="$config_vars
>  with-lld = $libc_cv_with_lld"
>  
> +
> +# Check whether --with-ld_relro_load_gaps was given.
> +if test ${with_ld_relro_load_gaps+y}
> +then :
> +  withval=$with_ld_relro_load_gaps;
> +else case e in #(
> +  e) :  ;;
> +esac
> +fi
> +
> +if test "x$with_ld_relro_load_gaps" = xcheck
> +then :
> +  echo "Checking binutils ld version:" >&5
> +   if LC_ALL=C $LD --version | grep -E '^GNU ld version 2\.(2[0-9]|3[0-8])[^0-9]' >&5
> +then :
> +  with_ld_relro_load_gaps=yes
> +else case e in #(
> +  e) with_ld_relro_load_gaps=no
> +	 echo "(linker not binutils or not impacted)" >&5 ;;
> +esac
> +fi
> +fi
> +if test "x$with_ld_relro_load_gaps" != xyes && test "x$with_ld_relro_load_gaps" != xno
> +then :
> +  as_fn_error $? "invalid --with-ld-load-gaps argument: $with_ld_relro_load_gaps" "$LINENO" 5
> +fi
> +if test "x$with_ld_relro_load_gaps" = xyes
> +then :
> +  printf "%s\n" "#define HAVE_LD_RELRO_LOAD_GAPS 1" >>confdefs.h
> +
> +fi
> +config_vars="$config_vars
> +have-ld-relro-load-gaps = $with_ld_relro_load_gaps"
> +
>  # These programs are version sensitive.
>  for ac_prog in gnumake gmake make
>  do
> diff --git a/configure.ac b/configure.ac
> index 7c9b57789e..e4a3ff98a4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -535,6 +535,26 @@ case $($LD --version) in
>  esac
>  LIBC_CONFIG_VAR([with-lld], [$libc_cv_with_lld])
>  
> +dnl Workaround for binutils LOAD segment gaps bug (swbz#28743)
> +dnl Fixed in commit 9833b7757d246f22db4eb24b8e5db7eb5e05b6d9
> +dnl ("PR28824, relro security issues"), part of binutils 2.39.
> +AC_ARG_WITH([ld_relro_load_gaps],
> +  [AS_HELP_STRING([--with-ld-relro-load-gaps],
> +    [support linker with RELRO LOAD segment gap bug @<:@default=check@:>@])],
> +  [],
> +  [: m4_divert_text([DEFAULTS], [with_ld_relro_load_gaps=check])])
> +AS_IF([test "x$with_ld_relro_load_gaps" = xcheck],
> +  [echo "Checking binutils ld version:" >&AS_MESSAGE_LOG_FD
> +   AS_IF([LC_ALL=C $LD --version | grep -E '^GNU ld version 2\.(2[[0-9]]|3[[0-8]])[[^0-9]]' >&AS_MESSAGE_LOG_FD],
> +        [with_ld_relro_load_gaps=yes],
> +	[with_ld_relro_load_gaps=no
> +	 echo "(linker not binutils or not impacted)" >&AS_MESSAGE_LOG_FD])])
> +AS_IF([test "x$with_ld_relro_load_gaps" != xyes && test "x$with_ld_relro_load_gaps" != xno],
> +  AC_MSG_ERROR([invalid --with-ld-load-gaps argument: $with_ld_relro_load_gaps]))
> +AS_IF([test "x$with_ld_relro_load_gaps" = xyes],
> +  [AC_DEFINE(HAVE_LD_RELRO_LOAD_GAPS)])
> +LIBC_CONFIG_VAR([have-ld-relro-load-gaps], [$with_ld_relro_load_gaps])
> +
>  # These programs are version sensitive.
>  AC_CHECK_PROG_VER(MAKE, gnumake gmake make, --version,
>    [GNU Make[^0-9]*\([0-9][0-9.]*\)],
> diff --git a/elf/Makefile b/elf/Makefile
> index 77e211b821..03ae0a34ea 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -627,6 +627,31 @@ $(objpfx)tst-relro-libc.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
>  	    --required=__io_vtables \
>  	  > $@ 2>&1; $(evaluate-test)
>  
> +# Only test ld.so because libc.so et al. are mapped by ld.so, which
> +# avoids creating gaps even if they are present in the ELF file.
> +tests-special += $(objpfx)tst-gaps-ldso.out
> +tst-program-headers-invocation = \
> +  $(PYTHON) tst-program-headers.py \
> +  --cc="$(CC) $(patsubst -DMODULE_NAME=%,-DMODULE_NAME=testsuite,$(CPPFLAGS))"
> +$(objpfx)tst-gaps-ldso.out: tst-program-headers.py \
> +  $(..)/scripts/glibcelf.py $(objpfx)ld.so
> +	$(tst-program-headers-invocation) --mode=gaps $(objpfx)ld.so \
> +	  > $@ 2>&1; $(evaluate-test)
> +ifeq ($(have-ld-relro-load-gaps),yes)
> +# This test may fail if the linker has bug swbz#28743.
> +test-xfail-tst-gaps-ldso = yes
> +else
> +# Otherwise this test fails if the architecture supports multiple page sizes.
> +test-xfail-tst-gaps-ldso = $(shell $(tst-program-headers-invocation) --mode=gaps-xfail $(objpfx)ld.so)
> +endif
> +
> +tests-special += $(objpfx)tst-load-alignment.out
> +$(objpfx)tst-load-alignment.out: tst-program-headers.py \
> +  $(..)/scripts/glibcelf.py $(objpfx)ld.so $(common-objpfx)libc.so
> +	$(tst-program-headers-invocation) --mode=alignment \
> +	  $(objpfx)ld.so $(common-objpfx)libc.so \
> +	  > $@ 2>&1; $(evaluate-test)
> +
>  ifeq ($(run-built-tests),yes)
>  tests-special += $(objpfx)tst-valgrind-smoke.out
>  endif
> diff --git a/elf/tst-program-headers.py b/elf/tst-program-headers.py
> new file mode 100644
> index 0000000000..1ea8982def
> --- /dev/null
> +++ b/elf/tst-program-headers.py
> @@ -0,0 +1,137 @@
> +#!/usr/bin/python3
> +# Verify properties of ELF program headers.
> +# Copyright (C) 2024 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +#
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
> +
> +import argparse
> +import os.path
> +import sys
> +
> +# Make available glibc Python modules.
> +sys.path.append(os.path.join(
> +    os.path.dirname(os.path.realpath(__file__)), os.path.pardir, 'scripts'))
> +
> +import glibcelf
> +import glibcextract
> +
> +def rounddown(val, align):
> +    """Round down VAL to a multiple of ALIGN (which is a power of two)."""
> +    assert (align & (align - 1)) == 0, align
> +    return val & -align
> +
> +def roundup(val, align):
> +    """Round up VAL to a multiple of ALIGN (which is a power of two)."""
> +    assert (align & (align - 1)) == 0, align
> +    return (val + align - 1) & -align
> +
> +errors_encountered = 0
> +
> +def init_page_sizes(cc):
> +    """Initializes page_size_min and page_size_max."""
> +    consts = glibcextract.compute_macro_consts(
> +        source_text='#include <sys/pagesize.h>',
> +        cc=cc,
> +        macro_re='^PAGE_SIZE_.*$')
> +    global page_size_min
> +    page_size_min = int(consts['PAGE_SIZE_MIN'])
> +    global page_size_max
> +    page_size_max = int(consts['PAGE_SIZE_MAX'])
> +    if page_size_min > page_size_max:
> +        print('error: minimum page size {} is greater than maximum page size'
> +              .format(page_size_min, page_size_max))
> +
> +def check_variant_gaps(path, img):
> +    """Check that there are no gaps between load segments."""
> +    print('info: mininum page size:', page_size_min)
> +    global errors_encountered
> +    loads = [phdr for phdr in img.phdrs()
> +             if phdr.p_type == glibcelf.Pt.PT_LOAD]
> +    if not loads:
> +        # Nothing to check.
> +        return
> +    current_address = None
> +    for idx, phdr in enumerate(loads):
> +        this_address = rounddown(phdr.p_vaddr, page_size_min)
> +        next_address = roundup(phdr.p_vaddr + phdr.p_memsz, page_size_min)
> +        print('info: {}: LOAD segment {}: address 0x{:x}, size {},'
> +              ' range [0x{:x},0x{:x})'
> +               .format(path, idx, phdr.p_vaddr, phdr.p_memsz,
> +                       this_address, next_address))
> +        if current_address is not None:
> +            gap = this_address - current_address
> +            if gap != 0:
> +                errors_encountered += 1
> +                print('error: {}: gap between load segments: {} bytes'.format(
> +                    path, gap))
> +        current_address = next_address
> +
> +def check_variant_gaps_xfail(path, img):
> +    """Print 'yes' if the gaps test is expected to fail."""
> +    # This is not an actual check.
> +    if page_size_min != page_size_max:
> +        print('yes')
> +
> +def check_variant_alignment(path, img):
> +    """Check that the binary can be loaded with the maximum page size."""
> +    print('info: maximum page size:', page_size_max)
> +    global errors_encountered
> +    for phdr in img.phdrs():
> +        if phdr.p_type != glibcelf.Pt.PT_LOAD:
> +            continue
> +        page_file_offset = phdr.p_offset % page_size_max
> +        page_virtual_offset = phdr.p_vaddr % page_size_max
> +        if page_file_offset != page_virtual_offset:
> +            print('error: {}: LOAD segment {} cannot be loaded for page size {}'
> +                  .format(path, phdr, page_size_max))
> +            print('note: file offset in page:', page_file_offset)
> +            print('note: virtual offset:', page_virtual_offset)
> +            errors_encountered += 1
> +
> +def main():
> +    """The main entry point."""
> +    parser = argparse.ArgumentParser(
> +        description="Test script for analyzing ELF progam headers")
> +    parser.add_argument('--cc', metavar='CC',
> +                        help='C compiler (including options to use)')
> +    parser.add_argument('--mode', metavar='MODE',
> +                        choices='gaps gaps-xfail alignment'.split(),
> +                        help='ELF program header analysis to carry out')
> +    parser.add_argument('objects', metavar='PATH', nargs='*',
> +                        help='Main programs and shared objects to process')
> +    args = parser.parse_args()
> +
> +    global errors_encountered
> +
> +    if not args.cc:
> +        print('error: missing --cc argument')
> +        errors_encountered += 1
> +    if not args.mode:
> +        print('error: missing --mode argument')
> +        errors_encountered += 1
> +
> +    if errors_encountered == 0:
> +        init_page_sizes(args.cc)
> +        check = globals()['check_variant_' + args.mode.replace('-', '_')]
> +        for path in args.objects:
> +            check(path, glibcelf.Image.readfile(path))
> +
> +    if errors_encountered > 0:
> +        print('note: errors encountered:', errors_encountered)
> +        sys.exit(1)
> +
> +if __name__ == '__main__':
> +    main()
> diff --git a/manual/install.texi b/manual/install.texi
> index 3e68a3d823..e37bb958c1 100644
> --- a/manual/install.texi
> +++ b/manual/install.texi
> @@ -117,6 +117,18 @@ problem and suppress these constructs, so that the library will still be
>  usable, but functionality may be lost---for example, you can't build a
>  shared libc with old binutils.
>  
> +@item --with-ld-relro-load-gaps=@var{choice}
> +If @var{choice} is @samp{yes}, assume that the linker may produce gaps
> +between LOAD segments when linking @code{ld.so}, related to RELRO
> +processing.  With @samp{no}, assume that no such gaps are produced.  The
> +default for @var{choice} is @samp{check}, which performs a version-only
> +linker check.  The binutils linker bug in question is
> +@url{https://sourceware.org/bugzilla/show_bug.cgi?id=28743}.
> +
> +There is another source of LOAD segment gaps on architectures which
> +support multiple page sizes.  The @option{--with-ld-relro-load-gaps} is
> +not related to that.
> +
>  @item --with-nonshared-cflags=@var{cflags}
>  Use additional compiler flags @var{cflags} to build the parts of the
>  library which are always statically linked into applications and



More information about the Libc-alpha mailing list