diff mbox series

[v3,2/2] Default to --with-default-link=no (bug 25812)

Message ID 15c7f6e9afe2c2b9c51ebc6372682a39b0932712.1649691083.git.fweimer@redhat.com
State New
Headers show
Series [v3,1/2] scripts: Add glibcelf.py module | expand

Commit Message

Florian Weimer April 11, 2022, 3:32 p.m. UTC
This is necessary to place the libio vtables into the RELRO segment.
New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
verify that this is what actually happens.

The new tests fail on ia64 due to lack of (default) RELRO support
inbutils, so they are XFAILed there.
---
v3: Fix INSTALL type and update wording.
 INSTALL                               |   6 ++
 configure                             |  65 +-----------
 configure.ac                          |  55 +----------
 elf/Makefile                          |  33 +++++++
 elf/tst-relro-symbols.py              | 137 ++++++++++++++++++++++++++
 manual/install.texi                   |   6 ++
 sysdeps/unix/sysv/linux/ia64/Makefile |   6 ++
 7 files changed, 190 insertions(+), 118 deletions(-)
 create mode 100644 elf/tst-relro-symbols.py

Comments

Siddhesh Poyarekar April 22, 2022, 6:12 a.m. UTC | #1
On 11/04/2022 21:02, Florian Weimer via Libc-alpha wrote:
> This is necessary to place the libio vtables into the RELRO segment.
> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
> verify that this is what actually happens.
> 
> The new tests fail on ia64 due to lack of (default) RELRO support
> inbutils, so they are XFAILed there.
> ---
> v3: Fix INSTALL type and update wording.
>   INSTALL                               |   6 ++
>   configure                             |  65 +-----------
>   configure.ac                          |  55 +----------
>   elf/Makefile                          |  33 +++++++
>   elf/tst-relro-symbols.py              | 137 ++++++++++++++++++++++++++
>   manual/install.texi                   |   6 ++
>   sysdeps/unix/sysv/linux/ia64/Makefile |   6 ++
>   7 files changed, 190 insertions(+), 118 deletions(-)
>   create mode 100644 elf/tst-relro-symbols.py
> 
> diff --git a/INSTALL b/INSTALL
> index 63c022d6b9..de150f66eb 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -90,6 +90,12 @@ 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-default-link=FLAG'
> +     With '--with-default-link=yes', the GNU C Library does not use a
> +     custom linker scipt for linking its individual shared objects.  The
> +     default for FLAG is the opposite, 'no', because the custom linker
> +     script is needed for full RELRO protection.
> +

Andreas' comments still apply here I think, i.e. fix the "scipt" type 
and rephrase so that it's clearer that the option controls the library 
build process and not the library itself.

Does it make sense to make this --disable-custom-link or 
--enable-default-link instead, since the option is binary?  The --with 
prefix is typically for richer options, e.g. paths.  Suggest something 
like this:

--disable-custom-link
     Don't use a custom linker script to build the GNU C Library,
     preferring the static linker's default script instead.  The custom
     linker script is needed for full RELRO protection.

>   '--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/configure b/configure
> index d2f413d05d..650bfd982c 100755
> --- a/configure
> +++ b/configure
> @@ -3375,7 +3375,7 @@ fi
>   if test "${with_default_link+set}" = set; then :
>     withval=$with_default_link; use_default_link=$withval
>   else
> -  use_default_link=default
> +  use_default_link=no
>   fi

... and then maybe adjust variable names accordingly.

>   
>   
> @@ -6184,69 +6184,6 @@ fi
>   $as_echo "$libc_cv_hashstyle" >&6; }
>   
>   
> -# The linker's default -shared behavior is good enough if it
> -# does these things that our custom linker scripts ensure that
> -# all allocated NOTE sections come first.
> -if test "$use_default_link" = default; then
> -  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for sufficient default -shared layout" >&5
> -$as_echo_n "checking for sufficient default -shared layout... " >&6; }
> -if ${libc_cv_use_default_link+:} false; then :
> -  $as_echo_n "(cached) " >&6
> -else
> -    libc_cv_use_default_link=no
> -  cat > conftest.s <<\EOF
> -	  .section .note.a,"a",%note
> -	  .balign 4
> -	  .long 4,4,9
> -	  .string "GNU"
> -	  .string "foo"
> -	  .section .note.b,"a",%note
> -	  .balign 4
> -	  .long 4,4,9
> -	  .string "GNU"
> -	  .string "bar"
> -EOF
> -  if { ac_try='  ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&5'
> -  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> -  (eval $ac_try) 2>&5
> -  ac_status=$?
> -  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> -  test $ac_status = 0; }; } &&
> -       ac_try=`$READELF -S conftest.so | sed -n \
> -	 '${x;p;}
> -	  s/^ *\[ *[1-9][0-9]*\]  *\([^ ][^ ]*\)  *\([^ ][^ ]*\) .*$/\2 \1/
> -	  t a
> -	  b
> -	  : a
> -	  H'`
> -  then
> -    libc_seen_a=no libc_seen_b=no
> -    set -- $ac_try
> -    while test $# -ge 2 -a "$1" = NOTE; do
> -      case "$2" in
> -      .note.a) libc_seen_a=yes ;;
> -      .note.b) libc_seen_b=yes ;;
> -      esac
> -      shift 2
> -    done
> -    case "$libc_seen_a$libc_seen_b" in
> -    yesyes)
> -      libc_cv_use_default_link=yes
> -      ;;
> -    *)
> -      echo >&5 "\
> -$libc_seen_a$libc_seen_b from:
> -$ac_try"
> -      ;;
> -    esac
> -  fi
> -  rm -f conftest*
> -fi
> -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_use_default_link" >&5
> -$as_echo "$libc_cv_use_default_link" >&6; }
> -  use_default_link=$libc_cv_use_default_link
> -fi
> -
>   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for GLOB_DAT reloc" >&5
>   $as_echo_n "checking for GLOB_DAT reloc... " >&6; }
>   if ${libc_cv_has_glob_dat+:} false; then :
> diff --git a/configure.ac b/configure.ac
> index b6a747dece..605efd549d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -153,7 +153,7 @@ AC_ARG_WITH([default-link],
>   	    AS_HELP_STRING([--with-default-link],
>   			   [do not use explicit linker scripts]),
>   	    [use_default_link=$withval],
> -	    [use_default_link=default])
> +	    [use_default_link=no])
>   
>   dnl Additional build flags injection.
>   AC_ARG_WITH([nonshared-cflags],
> @@ -1371,59 +1371,6 @@ fi
>   rm -f conftest*])
>   AC_SUBST(libc_cv_hashstyle)
>   
> -# The linker's default -shared behavior is good enough if it
> -# does these things that our custom linker scripts ensure that
> -# all allocated NOTE sections come first.
> -if test "$use_default_link" = default; then
> -  AC_CACHE_CHECK([for sufficient default -shared layout],
> -		  libc_cv_use_default_link, [dnl
> -  libc_cv_use_default_link=no
> -  cat > conftest.s <<\EOF
> -	  .section .note.a,"a",%note
> -	  .balign 4
> -	  .long 4,4,9
> -	  .string "GNU"
> -	  .string "foo"
> -	  .section .note.b,"a",%note
> -	  .balign 4
> -	  .long 4,4,9
> -	  .string "GNU"
> -	  .string "bar"
> -EOF
> -  if AC_TRY_COMMAND([dnl
> -  ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&AS_MESSAGE_LOG_FD]) &&
> -       ac_try=`$READELF -S conftest.so | sed -n \
> -	 ['${x;p;}
> -	  s/^ *\[ *[1-9][0-9]*\]  *\([^ ][^ ]*\)  *\([^ ][^ ]*\) .*$/\2 \1/
> -	  t a
> -	  b
> -	  : a
> -	  H']`
> -  then
> -    libc_seen_a=no libc_seen_b=no
> -    set -- $ac_try
> -    while test $# -ge 2 -a "$1" = NOTE; do
> -      case "$2" in
> -      .note.a) libc_seen_a=yes ;;
> -      .note.b) libc_seen_b=yes ;;
> -      esac
> -      shift 2
> -    done
> -    case "$libc_seen_a$libc_seen_b" in
> -    yesyes)
> -      libc_cv_use_default_link=yes
> -      ;;
> -    *)
> -      echo >&AS_MESSAGE_LOG_FD "\
> -$libc_seen_a$libc_seen_b from:
> -$ac_try"
> -      ;;
> -    esac
> -  fi
> -  rm -f conftest*])
> -  use_default_link=$libc_cv_use_default_link
> -fi
> -
>   AC_CACHE_CHECK(for GLOB_DAT reloc,
>   	       libc_cv_has_glob_dat, [dnl
>   cat > conftest.c <<EOF
> diff --git a/elf/Makefile b/elf/Makefile
> index c96924e9c2..366559fc96 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -543,6 +543,39 @@ endif
>   endif
>   endif
>   
> +tests-special += $(objpfx)tst-relro-ldso.out $(objpfx)tst-relro-libc.out
> +$(objpfx)tst-relro-ldso.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
> +  $(objpfx)ld.so
> +	$(PYTHON) tst-relro-symbols.py $(objpfx)ld.so \
> +	  --required=_rtld_global_ro \
> +	  > $@ 2>&1; $(evaluate-test)
> +# The optional symbols are present in libc only if the architecture has
> +# the GLIBC_2.0 symbol set in libc.
> +$(objpfx)tst-relro-libc.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
> +  $(common-objpfx)libc.so
> +	$(PYTHON) tst-relro-symbols.py $(common-objpfx)libc.so \
> +	    --required=_IO_cookie_jumps \
> +	    --required=_IO_file_jumps \
> +	    --required=_IO_file_jumps_maybe_mmap \
> +	    --required=_IO_file_jumps_mmap \
> +	    --required=_IO_helper_jumps \
> +	    --required=_IO_mem_jumps \
> +	    --required=_IO_obstack_jumps \
> +	    --required=_IO_proc_jumps \
> +	    --required=_IO_str_chk_jumps \
> +	    --required=_IO_str_jumps \
> +	    --required=_IO_strn_jumps \
> +	    --required=_IO_wfile_jumps \
> +	    --required=_IO_wfile_jumps_maybe_mmap \
> +	    --required=_IO_wfile_jumps_mmap \
> +	    --required=_IO_wmem_jumps \
> +	    --required=_IO_wstr_jumps \
> +	    --required=_IO_wstrn_jumps \
> +	    --optional=_IO_old_cookie_jumps \
> +	    --optional=_IO_old_file_jumps \
> +	    --optional=_IO_old_proc_jumps \
> +	  > $@ 2>&1; $(evaluate-test)
> +

Run the new test.  OK.  Verified that it passes with 
--with-default-link=no and fails with --with-default-link=yes.

>   ifeq ($(run-built-tests),yes)
>   tests-special += $(objpfx)tst-valgrind-smoke.out
>   endif
> diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
> new file mode 100644
> index 0000000000..368ea3349f
> --- /dev/null
> +++ b/elf/tst-relro-symbols.py
> @@ -0,0 +1,137 @@
> +#!/usr/bin/python3
> +# Verify that certain symbols are covered by RELRO.
> +# Copyright (C) 2022 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/>.
> +
> +"""Analyze a (shared) object to verify that certain symbols are
> +present and covered by the PT_GNU_RELRO segment.
> +
> +"""
> +
> +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'))

I wonder if we should put this in the python environment for all glibc 
python scripts.  Only a soft suggestion though since this is just the 
first module usage.  Maybe as and when we port other such tests to this 
module we should rethink this.

> +
> +import glibcelf
> +
> +def find_relro(path: str, img: glibcelf.Image) -> (int, int):
> +    """Discover the address range of the PT_GNU_RELRO segment."""
> +    for phdr in img.phdrs():
> +        if phdr.p_type == glibcelf.Pt.PT_GNU_RELRO:
> +            # The computation is not entirely accurate because
> +            # _dl_protect_relro in elf/dl-reloc.c rounds both the
> +            # start end and downwards using the run-time page size.
> +            return phdr.p_vaddr, phdr.p_vaddr + phdr.p_memsz
> +    sys.stdout.write('{}: error: no PT_GNU_RELRO segment\n'.format(path))
> +    sys.exit(1)

OK.

> +
> +def check_in_relro(kind, relro_begin, relro_end, name, start, size, error):
> +    """Check if a section or symbol falls within in the RELRO segment."""
> +    end = start + size - 1
> +    if not (relro_begin <= start < end < relro_end):
> +        error(
> +            '{} {!r} of size {} at 0x{:x} is not in RELRO range [0x{:x}, 0x{:x})'.format(
> +                kind, name.decode('UTF-8'), start, size,
> +                relro_begin, relro_end))

OK.  Some day this will be a reusable function I think; we may end up 
with more tests that want to check if some symbol is in relro.

> +
> +def get_parser():
> +    """Return an argument parser for this script."""
> +    parser = argparse.ArgumentParser(description=__doc__)
> +    parser.add_argument('object', help='path to object file to check')
> +    parser.add_argument('--required', metavar='NAME', default=(),
> +                        help='required symbol names', nargs='*')
> +    parser.add_argument('--optional', metavar='NAME', default=(),
> +                        help='required symbol names', nargs='*')
> +    return parser

OK.

> +
> +def main(argv):
> +    """The main entry point."""
> +    parser = get_parser()
> +    opts = parser.parse_args(argv)
> +    img = glibcelf.Image.readfile(opts.object)
> +
> +    required_symbols = frozenset([sym.encode('UTF-8')
> +                                  for sym in opts.required])
> +    optional_symbols = frozenset([sym.encode('UTF-8')
> +                                  for sym in opts.optional])
> +    check_symbols = required_symbols | optional_symbols
> +
> +    # Tracks the symbols in check_symbols that have been found.
> +    symbols_found = set()
> +
> +    # Discover the extent of the RELRO segment.
> +    relro_begin, relro_end = find_relro(opts.object, img)
> +    symbol_table_found = False
> +
> +    errors = False
> +    def error(msg: str) -> None:
> +        """Record an error condition and write a message to standard output."""
> +        nonlocal errors
> +        errors = True
> +        sys.stdout.write('{}: error: {}\n'.format(opts.object, msg))
> +
> +    # Iterate over section headers to find the symbol table.
> +    for shdr in img.shdrs():
> +        if shdr.sh_type == glibcelf.Sht.SHT_SYMTAB:
> +            symbol_table_found = True
> +            for sym in img.syms(shdr):
> +                if sym.st_name in check_symbols:
> +                    symbols_found.add(sym.st_name)
> +
> +                    # Validate symbol type, section, and size.
> +                    if sym.st_info.type != glibcelf.Stt.STT_OBJECT:
> +                        error('symbol {!r} has wrong type {}'.format(
> +                            sym.st_name.decode('UTF-8'), sym.st_info.type))
> +                    if sym.st_shndx in glibcelf.Shn:
> +                        error('symbol {!r} has reserved section {}'.format(
> +                            sym.st_name.decode('UTF-8'), sym.st_shndx))
> +                        continue
> +                    if sym.st_size == 0:
> +                        error('symbol {!r} has size zero'.format(
> +                            sym.st_name.decode('UTF-8')))
> +                        continue
> +
> +                    check_in_relro('symbol', relro_begin, relro_end,
> +                                   sym.st_name, sym.st_value, sym.st_size,
> +                                   error)
> +            continue # SHT_SYMTAB
> +        if shdr.sh_name == b'.data.rel.ro' \
> +           or shdr.sh_name.startswith(b'.data.rel.ro.'):
> +            check_in_relro('section', relro_begin, relro_end,
> +                           shdr.sh_name, shdr.sh_addr, shdr.sh_size,
> +                           error)
> +            continue
> +
> +    if required_symbols - symbols_found:
> +        for sym in sorted(required_symbols - symbols_found):
> +            error('symbol {!r} not found'.format(sym.decode('UTF-8')))
> +
> +    if errors:
> +        sys.exit(1)
> +
> +    if not symbol_table_found:
> +        sys.stdout.write(
> +            '{}: warning: no symbol table found (stripped object)\n'.format(
> +                opts.object))
> +        sys.exit(77)
> +
> +if __name__ == '__main__':
> +    main(sys.argv[1:])

OK.

> diff --git a/manual/install.texi b/manual/install.texi
> index 29c52f2927..fcfb6901e4 100644
> --- a/manual/install.texi
> +++ b/manual/install.texi
> @@ -117,6 +117,12 @@ 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-default-link=@var{FLAG}
> +With @code{--with-default-link=yes}, the build system does not use a
> +custom linker script for linking shared objects.  The default for
> +@var{FLAG} is the opposite, @samp{no}, because the custom linker script
> +is needed for full RELRO protection.
> +
>   @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
> diff --git a/sysdeps/unix/sysv/linux/ia64/Makefile b/sysdeps/unix/sysv/linux/ia64/Makefile
> index da85ba43e2..c5cc41b367 100644
> --- a/sysdeps/unix/sysv/linux/ia64/Makefile
> +++ b/sysdeps/unix/sysv/linux/ia64/Makefile
> @@ -1,3 +1,9 @@
> +ifeq ($(subdir),elf)
> +# ia64 does not support PT_GNU_RELRO.
> +test-xfail-tst-relro-ldso = yes
> +test-xfail-tst-relro-libc = yes
> +endif
> +
>   ifeq ($(subdir),misc)
>   sysdep_headers += sys/rse.h
>   endif

OK.
Florian Weimer April 22, 2022, 6:35 a.m. UTC | #2
* Siddhesh Poyarekar:

>> diff --git a/INSTALL b/INSTALL
>> index 63c022d6b9..de150f66eb 100644
>> --- a/INSTALL
>> +++ b/INSTALL
>> @@ -90,6 +90,12 @@ 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-default-link=FLAG'
>> +     With '--with-default-link=yes', the GNU C Library does not use a
>> +     custom linker scipt for linking its individual shared objects.  The
>> +     default for FLAG is the opposite, 'no', because the custom linker
>> +     script is needed for full RELRO protection.
>> +
>
> Andreas' comments still apply here I think, i.e. fix the "scipt" type
> and rephrase so that it's clearer that the option controls the library 
> build process and not the library itself.

I thought I had fixed this.  What about this?

'--with-default-link=FLAG'
     With '--with-default-link=yes', the build system does not use a
     custom linker script for linking shared objects.  The default for
     FLAG is the opposite, 'no', because the custom linker script is
     needed for full RELRO protection.

> Does it make sense to make this --disable-custom-link or
> --enable-default-link instead, since the option is binary?  The --with 
> prefix is typically for richer options, e.g. paths.  Suggest something
> like this:
>
> --disable-custom-link
>     Don't use a custom linker script to build the GNU C Library,
>     preferring the static linker's default script instead.  The custom
>     linker script is needed for full RELRO protection.

I want to backport this, and distributions are already using this
option, so I prefer not to make changes here.

>> diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
>> new file mode 100644
>> index 0000000000..368ea3349f
>> --- /dev/null
>> +++ b/elf/tst-relro-symbols.py
>> @@ -0,0 +1,137 @@

>> +# Make available glibc Python modules.
>> +sys.path.append(os.path.join(
>> +    os.path.dirname(os.path.realpath(__file__)), os.path.pardir, 'scripts'))
>
> I wonder if we should put this in the python environment for all glibc
> python scripts.  Only a soft suggestion though since this is just the 
> first module usage.  Maybe as and when we port other such tests to
> this module we should rethink this.

We set PYTHONPATH in the makefiles in many cases.  I did this mostly for
development.

Thanks,
Florian
Siddhesh Poyarekar April 22, 2022, 8:12 a.m. UTC | #3
On 22/04/2022 12:05, Florian Weimer wrote:
> * Siddhesh Poyarekar:
> 
>>> diff --git a/INSTALL b/INSTALL
>>> index 63c022d6b9..de150f66eb 100644
>>> --- a/INSTALL
>>> +++ b/INSTALL
>>> @@ -90,6 +90,12 @@ 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-default-link=FLAG'
>>> +     With '--with-default-link=yes', the GNU C Library does not use a
>>> +     custom linker scipt for linking its individual shared objects.  The
>>> +     default for FLAG is the opposite, 'no', because the custom linker
>>> +     script is needed for full RELRO protection.
>>> +
>>
>> Andreas' comments still apply here I think, i.e. fix the "scipt" type
>> and rephrase so that it's clearer that the option controls the library
>> build process and not the library itself.
> 
> I thought I had fixed this.  What about this?
> 
> '--with-default-link=FLAG'
>       With '--with-default-link=yes', the build system does not use a
>       custom linker script for linking shared objects.  The default for
>       FLAG is the opposite, 'no', because the custom linker script is
>       needed for full RELRO protection.

That looks better, thanks.

>> Does it make sense to make this --disable-custom-link or
>> --enable-default-link instead, since the option is binary?  The --with
>> prefix is typically for richer options, e.g. paths.  Suggest something
>> like this:
>>
>> --disable-custom-link
>>      Don't use a custom linker script to build the GNU C Library,
>>      preferring the static linker's default script instead.  The custom
>>      linker script is needed for full RELRO protection.
> 
> I want to backport this, and distributions are already using this
> option, so I prefer not to make changes here.

Fair enough.

>>> diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
>>> new file mode 100644
>>> index 0000000000..368ea3349f
>>> --- /dev/null
>>> +++ b/elf/tst-relro-symbols.py
>>> @@ -0,0 +1,137 @@
> 
>>> +# Make available glibc Python modules.
>>> +sys.path.append(os.path.join(
>>> +    os.path.dirname(os.path.realpath(__file__)), os.path.pardir, 'scripts'))
>>
>> I wonder if we should put this in the python environment for all glibc
>> python scripts.  Only a soft suggestion though since this is just the
>> first module usage.  Maybe as and when we port other such tests to
>> this module we should rethink this.
> 
> We set PYTHONPATH in the makefiles in many cases.  I did this mostly for
> development.

Indeed, in fact we have py-env in Rules which may be helpful.  How about 
using it the invocation instead of the above then?

Thanks,
Siddhesh
Florian Weimer April 22, 2022, 8:24 a.m. UTC | #4
* Siddhesh Poyarekar:

>>> I wonder if we should put this in the python environment for all glibc
>>> python scripts.  Only a soft suggestion though since this is just the
>>> first module usage.  Maybe as and when we port other such tests to
>>> this module we should rethink this.

>> We set PYTHONPATH in the makefiles in many cases.  I did this mostly
>> for development.
>
> Indeed, in fact we have py-env in Rules which may be helpful.  How
> about using it the invocation instead of the above then?

I do like that I can run the script directly. 8-)

Thanks,
Florian
Siddhesh Poyarekar April 22, 2022, 8:32 a.m. UTC | #5
On 22/04/2022 13:54, Florian Weimer wrote:
> * Siddhesh Poyarekar:
> 
>>>> I wonder if we should put this in the python environment for all glibc
>>>> python scripts.  Only a soft suggestion though since this is just the
>>>> first module usage.  Maybe as and when we port other such tests to
>>>> this module we should rethink this.
> 
>>> We set PYTHONPATH in the makefiles in many cases.  I did this mostly
>>> for development.
>>
>> Indeed, in fact we have py-env in Rules which may be helpful.  How
>> about using it the invocation instead of the above then?
> 
> I do like that I can run the script directly. 8-)

Fair enough :)

Siddhesh
Florian Weimer April 22, 2022, 8:34 a.m. UTC | #6
* Siddhesh Poyarekar:

> On 22/04/2022 13:54, Florian Weimer wrote:
>> * Siddhesh Poyarekar:
>> 
>>>>> I wonder if we should put this in the python environment for all glibc
>>>>> python scripts.  Only a soft suggestion though since this is just the
>>>>> first module usage.  Maybe as and when we port other such tests to
>>>>> this module we should rethink this.
>> 
>>>> We set PYTHONPATH in the makefiles in many cases.  I did this mostly
>>>> for development.
>>>
>>> Indeed, in fact we have py-env in Rules which may be helpful.  How
>>> about using it the invocation instead of the above then?
>> I do like that I can run the script directly. 8-)
>
> Fair enough :)

So it's good to install along with the glibcelf.py patch?

Thanks,
Florian
Siddhesh Poyarekar April 22, 2022, 8:36 a.m. UTC | #7
On 22/04/2022 14:04, Florian Weimer wrote:
> * Siddhesh Poyarekar:
> 
>> On 22/04/2022 13:54, Florian Weimer wrote:
>>> * Siddhesh Poyarekar:
>>>
>>>>>> I wonder if we should put this in the python environment for all glibc
>>>>>> python scripts.  Only a soft suggestion though since this is just the
>>>>>> first module usage.  Maybe as and when we port other such tests to
>>>>>> this module we should rethink this.
>>>
>>>>> We set PYTHONPATH in the makefiles in many cases.  I did this mostly
>>>>> for development.
>>>>
>>>> Indeed, in fact we have py-env in Rules which may be helpful.  How
>>>> about using it the invocation instead of the above then?
>>> I do like that I can run the script directly. 8-)
>>
>> Fair enough :)
> 
> So it's good to install along with the glibcelf.py patch?

Yes please.

Thanks,
Siddhesh
Andreas Schwab April 22, 2022, 8:56 a.m. UTC | #8
On Apr 22 2022, Florian Weimer via Libc-alpha wrote:

> '--with-default-link=FLAG'
>      With '--with-default-link=yes', the build system does not use a
>      custom linker script for linking shared objects.  The default for
>      FLAG is the opposite, 'no', because the custom linker script is
>      needed for full RELRO protection.

Since it's a binary option, just use --with-default-link and
--without-default-link.
Fangrui Song April 28, 2022, 7:25 a.m. UTC | #9
On 2022-04-11, Florian Weimer via Libc-alpha wrote:
>This is necessary to place the libio vtables into the RELRO segment.
>New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
>verify that this is what actually happens.
>
>The new tests fail on ia64 due to lack of (default) RELRO support
>inbutils, so they are XFAILed there.
>---
>v3: Fix INSTALL type and update wording.
> INSTALL                               |   6 ++
> configure                             |  65 +-----------
> configure.ac                          |  55 +----------
> elf/Makefile                          |  33 +++++++
> elf/tst-relro-symbols.py              | 137 ++++++++++++++++++++++++++
> manual/install.texi                   |   6 ++
> sysdeps/unix/sysv/linux/ia64/Makefile |   6 ++
> 7 files changed, 190 insertions(+), 118 deletions(-)
> create mode 100644 elf/tst-relro-symbols.py
>
>diff --git a/INSTALL b/INSTALL
>index 63c022d6b9..de150f66eb 100644
>--- a/INSTALL
>+++ b/INSTALL
>@@ -90,6 +90,12 @@ 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-default-link=FLAG'
>+     With '--with-default-link=yes', the GNU C Library does not use a
>+     custom linker scipt for linking its individual shared objects.  The
>+     default for FLAG is the opposite, 'no', because the custom linker
>+     script is needed for full RELRO protection.
>+
> '--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/configure b/configure
>index d2f413d05d..650bfd982c 100755
>--- a/configure
>+++ b/configure
>@@ -3375,7 +3375,7 @@ fi
> if test "${with_default_link+set}" = set; then :
>   withval=$with_default_link; use_default_link=$withval
> else
>-  use_default_link=default
>+  use_default_link=no
> fi
>
>
>@@ -6184,69 +6184,6 @@ fi
> $as_echo "$libc_cv_hashstyle" >&6; }
>
>
>-# The linker's default -shared behavior is good enough if it
>-# does these things that our custom linker scripts ensure that
>-# all allocated NOTE sections come first.
>-if test "$use_default_link" = default; then
>-  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for sufficient default -shared layout" >&5
>-$as_echo_n "checking for sufficient default -shared layout... " >&6; }
>-if ${libc_cv_use_default_link+:} false; then :
>-  $as_echo_n "(cached) " >&6
>-else
>-    libc_cv_use_default_link=no
>-  cat > conftest.s <<\EOF
>-	  .section .note.a,"a",%note
>-	  .balign 4
>-	  .long 4,4,9
>-	  .string "GNU"
>-	  .string "foo"
>-	  .section .note.b,"a",%note
>-	  .balign 4
>-	  .long 4,4,9
>-	  .string "GNU"
>-	  .string "bar"
>-EOF
>-  if { ac_try='  ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&5'
>-  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>-  (eval $ac_try) 2>&5
>-  ac_status=$?
>-  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>-  test $ac_status = 0; }; } &&
>-       ac_try=`$READELF -S conftest.so | sed -n \
>-	 '${x;p;}
>-	  s/^ *\[ *[1-9][0-9]*\]  *\([^ ][^ ]*\)  *\([^ ][^ ]*\) .*$/\2 \1/
>-	  t a
>-	  b
>-	  : a
>-	  H'`
>-  then
>-    libc_seen_a=no libc_seen_b=no
>-    set -- $ac_try
>-    while test $# -ge 2 -a "$1" = NOTE; do
>-      case "$2" in
>-      .note.a) libc_seen_a=yes ;;
>-      .note.b) libc_seen_b=yes ;;
>-      esac
>-      shift 2
>-    done
>-    case "$libc_seen_a$libc_seen_b" in
>-    yesyes)
>-      libc_cv_use_default_link=yes
>-      ;;
>-    *)
>-      echo >&5 "\
>-$libc_seen_a$libc_seen_b from:
>-$ac_try"
>-      ;;
>-    esac
>-  fi
>-  rm -f conftest*
>-fi
>-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_use_default_link" >&5
>-$as_echo "$libc_cv_use_default_link" >&6; }
>-  use_default_link=$libc_cv_use_default_link
>-fi
>-
> { $as_echo "$as_me:${as_lineno-$LINENO}: checking for GLOB_DAT reloc" >&5
> $as_echo_n "checking for GLOB_DAT reloc... " >&6; }
> if ${libc_cv_has_glob_dat+:} false; then :
>diff --git a/configure.ac b/configure.ac
>index b6a747dece..605efd549d 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -153,7 +153,7 @@ AC_ARG_WITH([default-link],
> 	    AS_HELP_STRING([--with-default-link],
> 			   [do not use explicit linker scripts]),
> 	    [use_default_link=$withval],
>-	    [use_default_link=default])
>+	    [use_default_link=no])
>
> dnl Additional build flags injection.
> AC_ARG_WITH([nonshared-cflags],
>@@ -1371,59 +1371,6 @@ fi
> rm -f conftest*])
> AC_SUBST(libc_cv_hashstyle)
>
>-# The linker's default -shared behavior is good enough if it
>-# does these things that our custom linker scripts ensure that
>-# all allocated NOTE sections come first.
>-if test "$use_default_link" = default; then
>-  AC_CACHE_CHECK([for sufficient default -shared layout],
>-		  libc_cv_use_default_link, [dnl
>-  libc_cv_use_default_link=no
>-  cat > conftest.s <<\EOF
>-	  .section .note.a,"a",%note
>-	  .balign 4
>-	  .long 4,4,9
>-	  .string "GNU"
>-	  .string "foo"
>-	  .section .note.b,"a",%note
>-	  .balign 4
>-	  .long 4,4,9
>-	  .string "GNU"
>-	  .string "bar"
>-EOF
>-  if AC_TRY_COMMAND([dnl
>-  ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&AS_MESSAGE_LOG_FD]) &&
>-       ac_try=`$READELF -S conftest.so | sed -n \
>-	 ['${x;p;}
>-	  s/^ *\[ *[1-9][0-9]*\]  *\([^ ][^ ]*\)  *\([^ ][^ ]*\) .*$/\2 \1/
>-	  t a
>-	  b
>-	  : a
>-	  H']`
>-  then
>-    libc_seen_a=no libc_seen_b=no
>-    set -- $ac_try
>-    while test $# -ge 2 -a "$1" = NOTE; do
>-      case "$2" in
>-      .note.a) libc_seen_a=yes ;;
>-      .note.b) libc_seen_b=yes ;;
>-      esac
>-      shift 2
>-    done
>-    case "$libc_seen_a$libc_seen_b" in
>-    yesyes)
>-      libc_cv_use_default_link=yes
>-      ;;
>-    *)
>-      echo >&AS_MESSAGE_LOG_FD "\
>-$libc_seen_a$libc_seen_b from:
>-$ac_try"
>-      ;;
>-    esac
>-  fi
>-  rm -f conftest*])
>-  use_default_link=$libc_cv_use_default_link
>-fi
>-
> AC_CACHE_CHECK(for GLOB_DAT reloc,
> 	       libc_cv_has_glob_dat, [dnl
> cat > conftest.c <<EOF
>diff --git a/elf/Makefile b/elf/Makefile
>index c96924e9c2..366559fc96 100644
>--- a/elf/Makefile
>+++ b/elf/Makefile
>@@ -543,6 +543,39 @@ endif
> endif
> endif
>
>+tests-special += $(objpfx)tst-relro-ldso.out $(objpfx)tst-relro-libc.out
>+$(objpfx)tst-relro-ldso.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
>+  $(objpfx)ld.so
>+	$(PYTHON) tst-relro-symbols.py $(objpfx)ld.so \
>+	  --required=_rtld_global_ro \
>+	  > $@ 2>&1; $(evaluate-test)
>+# The optional symbols are present in libc only if the architecture has
>+# the GLIBC_2.0 symbol set in libc.
>+$(objpfx)tst-relro-libc.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
>+  $(common-objpfx)libc.so
>+	$(PYTHON) tst-relro-symbols.py $(common-objpfx)libc.so \
>+	    --required=_IO_cookie_jumps \
>+	    --required=_IO_file_jumps \
>+	    --required=_IO_file_jumps_maybe_mmap \
>+	    --required=_IO_file_jumps_mmap \
>+	    --required=_IO_helper_jumps \
>+	    --required=_IO_mem_jumps \
>+	    --required=_IO_obstack_jumps \
>+	    --required=_IO_proc_jumps \
>+	    --required=_IO_str_chk_jumps \
>+	    --required=_IO_str_jumps \
>+	    --required=_IO_strn_jumps \
>+	    --required=_IO_wfile_jumps \
>+	    --required=_IO_wfile_jumps_maybe_mmap \
>+	    --required=_IO_wfile_jumps_mmap \
>+	    --required=_IO_wmem_jumps \
>+	    --required=_IO_wstr_jumps \
>+	    --required=_IO_wstrn_jumps \
>+	    --optional=_IO_old_cookie_jumps \
>+	    --optional=_IO_old_file_jumps \
>+	    --optional=_IO_old_proc_jumps \
>+	  > $@ 2>&1; $(evaluate-test)
>+
> ifeq ($(run-built-tests),yes)
> tests-special += $(objpfx)tst-valgrind-smoke.out
> endif
>diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
>new file mode 100644
>index 0000000000..368ea3349f
>--- /dev/null
>+++ b/elf/tst-relro-symbols.py
>@@ -0,0 +1,137 @@
>+#!/usr/bin/python3
>+# Verify that certain symbols are covered by RELRO.
>+# Copyright (C) 2022 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/>.
>+
>+"""Analyze a (shared) object to verify that certain symbols are
>+present and covered by the PT_GNU_RELRO segment.
>+
>+"""
>+
>+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
>+
>+def find_relro(path: str, img: glibcelf.Image) -> (int, int):
>+    """Discover the address range of the PT_GNU_RELRO segment."""
>+    for phdr in img.phdrs():
>+        if phdr.p_type == glibcelf.Pt.PT_GNU_RELRO:
>+            # The computation is not entirely accurate because
>+            # _dl_protect_relro in elf/dl-reloc.c rounds both the
>+            # start end and downwards using the run-time page size.
>+            return phdr.p_vaddr, phdr.p_vaddr + phdr.p_memsz
>+    sys.stdout.write('{}: error: no PT_GNU_RELRO segment\n'.format(path))
>+    sys.exit(1)
>+
>+def check_in_relro(kind, relro_begin, relro_end, name, start, size, error):
>+    """Check if a section or symbol falls within in the RELRO segment."""
>+    end = start + size - 1
>+    if not (relro_begin <= start < end < relro_end):
>+        error(
>+            '{} {!r} of size {} at 0x{:x} is not in RELRO range [0x{:x}, 0x{:x})'.format(
>+                kind, name.decode('UTF-8'), start, size,
>+                relro_begin, relro_end))
>+
>+def get_parser():
>+    """Return an argument parser for this script."""
>+    parser = argparse.ArgumentParser(description=__doc__)
>+    parser.add_argument('object', help='path to object file to check')
>+    parser.add_argument('--required', metavar='NAME', default=(),
>+                        help='required symbol names', nargs='*')
>+    parser.add_argument('--optional', metavar='NAME', default=(),
>+                        help='required symbol names', nargs='*')
>+    return parser
>+
>+def main(argv):
>+    """The main entry point."""
>+    parser = get_parser()
>+    opts = parser.parse_args(argv)
>+    img = glibcelf.Image.readfile(opts.object)
>+
>+    required_symbols = frozenset([sym.encode('UTF-8')
>+                                  for sym in opts.required])
>+    optional_symbols = frozenset([sym.encode('UTF-8')
>+                                  for sym in opts.optional])
>+    check_symbols = required_symbols | optional_symbols
>+
>+    # Tracks the symbols in check_symbols that have been found.
>+    symbols_found = set()
>+
>+    # Discover the extent of the RELRO segment.
>+    relro_begin, relro_end = find_relro(opts.object, img)
>+    symbol_table_found = False
>+
>+    errors = False
>+    def error(msg: str) -> None:
>+        """Record an error condition and write a message to standard output."""
>+        nonlocal errors
>+        errors = True
>+        sys.stdout.write('{}: error: {}\n'.format(opts.object, msg))
>+
>+    # Iterate over section headers to find the symbol table.
>+    for shdr in img.shdrs():
>+        if shdr.sh_type == glibcelf.Sht.SHT_SYMTAB:
>+            symbol_table_found = True
>+            for sym in img.syms(shdr):
>+                if sym.st_name in check_symbols:
>+                    symbols_found.add(sym.st_name)
>+
>+                    # Validate symbol type, section, and size.
>+                    if sym.st_info.type != glibcelf.Stt.STT_OBJECT:
>+                        error('symbol {!r} has wrong type {}'.format(
>+                            sym.st_name.decode('UTF-8'), sym.st_info.type))
>+                    if sym.st_shndx in glibcelf.Shn:
>+                        error('symbol {!r} has reserved section {}'.format(
>+                            sym.st_name.decode('UTF-8'), sym.st_shndx))
>+                        continue
>+                    if sym.st_size == 0:
>+                        error('symbol {!r} has size zero'.format(
>+                            sym.st_name.decode('UTF-8')))
>+                        continue
>+
>+                    check_in_relro('symbol', relro_begin, relro_end,
>+                                   sym.st_name, sym.st_value, sym.st_size,
>+                                   error)
>+            continue # SHT_SYMTAB
>+        if shdr.sh_name == b'.data.rel.ro' \
>+           or shdr.sh_name.startswith(b'.data.rel.ro.'):
>+            check_in_relro('section', relro_begin, relro_end,
>+                           shdr.sh_name, shdr.sh_addr, shdr.sh_size,
>+                           error)
>+            continue
>+
>+    if required_symbols - symbols_found:
>+        for sym in sorted(required_symbols - symbols_found):
>+            error('symbol {!r} not found'.format(sym.decode('UTF-8')))
>+
>+    if errors:
>+        sys.exit(1)
>+
>+    if not symbol_table_found:
>+        sys.stdout.write(
>+            '{}: warning: no symbol table found (stripped object)\n'.format(
>+                opts.object))
>+        sys.exit(77)
>+
>+if __name__ == '__main__':
>+    main(sys.argv[1:])
>diff --git a/manual/install.texi b/manual/install.texi
>index 29c52f2927..fcfb6901e4 100644
>--- a/manual/install.texi
>+++ b/manual/install.texi
>@@ -117,6 +117,12 @@ 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-default-link=@var{FLAG}
>+With @code{--with-default-link=yes}, the build system does not use a
>+custom linker script for linking shared objects.  The default for
>+@var{FLAG} is the opposite, @samp{no}, because the custom linker script
>+is needed for full RELRO protection.
>+
> @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
>diff --git a/sysdeps/unix/sysv/linux/ia64/Makefile b/sysdeps/unix/sysv/linux/ia64/Makefile
>index da85ba43e2..c5cc41b367 100644
>--- a/sysdeps/unix/sysv/linux/ia64/Makefile
>+++ b/sysdeps/unix/sysv/linux/ia64/Makefile
>@@ -1,3 +1,9 @@
>+ifeq ($(subdir),elf)
>+# ia64 does not support PT_GNU_RELRO.
>+test-xfail-tst-relro-ldso = yes
>+test-xfail-tst-relro-libc = yes
>+endif
>+
> ifeq ($(subdir),misc)
> sysdep_headers += sys/rse.h
> endif
>-- 
>2.35.1
>

I use chromium's prebuilt clang/lld and do something like:

   curl -s https://raw.githubusercontent.com/chromium/chromium/main/tools/clang/scripts/update.py | python3 - --output-dir=~/Stable
   sudo ln -sf ~/Stable/bin/lld /usr/local/bin/ld

so that gcc will use ld.lld.

ld.lld doesn't have a built-in linker script (ld.bfd --verbose), so $@T will be empty and `test -s $@T` will fail.
I wonder how to better support ld.lld with the default --with-default-link=no ...


Besides, the position of .hash in --with-default-link=no seems weird.
71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
of the RO segment.
Florian Weimer April 28, 2022, 8:40 a.m. UTC | #10
* Fangrui Song:

> I use chromium's prebuilt clang/lld and do something like:
>
>   curl -s https://raw.githubusercontent.com/chromium/chromium/main/tools/clang/scripts/update.py | python3 - --output-dir=~/Stable
>   sudo ln -sf ~/Stable/bin/lld /usr/local/bin/ld
>
> so that gcc will use ld.lld.
>
> ld.lld doesn't have a built-in linker script (ld.bfd --verbose), so $@T will be empty and `test -s $@T` will fail.
> I wonder how to better support ld.lld with the default --with-default-link=no ...

How does lld place sections without a linker script?  Purely based on
names?

> Besides, the position of .hash in --with-default-link=no seems weird.
> 71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
> of the RO segment.

I think it's odd that we have an old-style hash table at all.  Do we
really need it?  All tools should have migrated by now.

Thanks,
Florian
Fangrui Song April 29, 2022, 6 a.m. UTC | #11
On 2022-04-28, Florian Weimer wrote:
>* Fangrui Song:
>
>> I use chromium's prebuilt clang/lld and do something like:
>>
>>   curl -s https://raw.githubusercontent.com/chromium/chromium/main/tools/clang/scripts/update.py | python3 - --output-dir=~/Stable
>>   sudo ln -sf ~/Stable/bin/lld /usr/local/bin/ld
>>
>> so that gcc will use ld.lld.
>>
>> ld.lld doesn't have a built-in linker script (ld.bfd --verbose), so $@T will be empty and `test -s $@T` will fail.
>> I wonder how to better support ld.lld with the default --with-default-link=no ...
>
>How does lld place sections without a linker script?  Purely based on
>names?

It entirely uses code to describe the built-in rules like gold, while
GNU ld uses an internal linker script plus built-in code.
(some rules cannot be described by the linker script language and needs
code anyway).

There is no support dumping lld/gold's built-in rules into a linker
script.

I will add DATA_SEGMENT_RELRO_END support to ld.lld:
https://reviews.llvm.org/D124656 . With the patch, ld.lld can read shlib.lds
transformed from ld.bfd --verbose output.

I can think of 3 ways for lld:

* ask lld users to use --with-default-link=yes
* remove `test -s $@T` (if lld)
* add -fuse-ld=bfd

>> Besides, the position of .hash in --with-default-link=no seems weird.
>> 71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
>> of the RO segment.
>
>I think it's odd that we have an old-style hash table at all.  Do we
>really need it?  All tools should have migrated by now.

We can remove .hash for non-mips architectures.

(I am a bit sad .MIPS.xhash support went in, but thankfully it doesn't take
too much code.  The translation table could be solved in a better way. I
suspect the whole thing is unnecessary with RELR.)
Florian Weimer April 29, 2022, 7:09 a.m. UTC | #12
* Fangrui Song:

>>How does lld place sections without a linker script?  Purely based on
>>names?
>
> It entirely uses code to describe the built-in rules like gold, while
> GNU ld uses an internal linker script plus built-in code.
> (some rules cannot be described by the linker script language and needs
> code anyway).

How can we make sure that certain sections are covered by RELRO, and
still get start/stop symbols for them?

>>> Besides, the position of .hash in --with-default-link=no seems weird.
>>> 71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
>>> of the RO segment.
>>
>>I think it's odd that we have an old-style hash table at all.  Do we
>>really need it?  All tools should have migrated by now.
>
> We can remove .hash for non-mips architectures.

We need the support in glibc for most architectures in the dynamic
linker, but we don't have to build glibc with the legacy hash tables.

> (I am a bit sad .MIPS.xhash support went in, but thankfully it doesn't take
> too much code.  The translation table could be solved in a better way. I
> suspect the whole thing is unnecessary with RELR.)

Huh?  Aren't they unrelated?

Thanks,
Florian
Fangrui Song April 29, 2022, 7:43 a.m. UTC | #13
On 2022-04-29, Florian Weimer wrote:
>* Fangrui Song:
>
>>>How does lld place sections without a linker script?  Purely based on
>>>names?
>>
>> It entirely uses code to describe the built-in rules like gold, while
>> GNU ld uses an internal linker script plus built-in code.
>> (some rules cannot be described by the linker script language and needs
>> code anyway).
>
>How can we make sure that certain sections are covered by RELRO, and
>still get start/stop symbols for them?

About __start_/__stop_:
__start_/__stop_ symbols are special. They don't need to be mentioned in a
linker script to take effects for lld and modern GNU ld. I haven't checked
whether there is an ancient GNU ld which requires symbol assignments when the
output section description is specified.

About RELRO:
It seems that there is no section name convention to make a section
RELRO.  Having a SECTIONS command with DATA_SEGMENT_ALIGN /
DATA_SEGMENT_RELRO_END can specify all RELRO sections. I do not know a
simpler approach than postprocessing ld.bfd --verbose output.

To make GNU ld work, the following linker script is sufficient:

   SECTIONS {
            __libc_subfreeres : { *(__libc_subfreeres) }
            __libc_atexit : { *(__libc_atexit) }
            __libc_IO_vtables : { *(__libc_IO_vtables) }
   } INSERT BEFORE .data.rel.ro;

This script does not work with ld.lld right now because there is an
`error: section ... is not contiguous with other relro sections` from
https://reviews.llvm.org/D40359 . I need to think whether the diagnostic
can be suppressed in some cases.

>>>> Besides, the position of .hash in --with-default-link=no seems weird.
>>>> 71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
>>>> of the RO segment.
>>>
>>>I think it's odd that we have an old-style hash table at all.  Do we
>>>really need it?  All tools should have migrated by now.
>>
>> We can remove .hash for non-mips architectures.
>
>We need the support in glibc for most architectures in the dynamic
>linker, but we don't have to build glibc with the legacy hash tables.

Agree

>> (I am a bit sad .MIPS.xhash support went in, but thankfully it doesn't take
>> too much code.  The translation table could be solved in a better way. I
>> suspect the whole thing is unnecessary with RELR.)
>
>Huh?  Aren't they unrelated?
>

`DT_MIPS_LOCAL_GOTNO` and `DT_MIPS_SYMTABNO-DT_MIPS_GOTSYM` can be
defined in a way that there is no special handling. Then there will be no
requirement on the .dynsym ordering. Then the regular DT_GNU_HASH can be
used, and DT_MIPS_XHASH will be unneeded.

`DT_MIPS_LOCAL_GOTNO` optimizes on top of relative relocations but with RELR
the additional benefit is very low.

>Thanks,
>Florian
>
Adhemerval Zanella Netto April 29, 2022, 12:59 p.m. UTC | #14
On 11/04/2022 12:32, Florian Weimer via Libc-alpha wrote:
> This is necessary to place the libio vtables into the RELRO segment.
> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
> verify that this is what actually happens.
> 
> The new tests fail on ia64 due to lack of (default) RELRO support
> inbutils, so they are XFAILed there.

I always frown upon new configure options, since they tend to degrade over
time (as --disable-shared for instance [1]) and require additional 
maintaining costs.  Why can't we make the required semantic the default 
(since usually relro has minimal costs it a net gain in hardening)?

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=20845
Florian Weimer April 29, 2022, 1:17 p.m. UTC | #15
* Adhemerval Zanella:

> On 11/04/2022 12:32, Florian Weimer via Libc-alpha wrote:
>> This is necessary to place the libio vtables into the RELRO segment.
>> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
>> verify that this is what actually happens.
>> 
>> The new tests fail on ia64 due to lack of (default) RELRO support
>> inbutils, so they are XFAILed there.
>
> I always frown upon new configure options, since they tend to degrade over
> time (as --disable-shared for instance [1]) and require additional 
> maintaining costs.  Why can't we make the required semantic the default 
> (since usually relro has minimal costs it a net gain in hardening)?

This is what the patch does, I think.

Thanks,
Florian
Adhemerval Zanella Netto April 29, 2022, 1:55 p.m. UTC | #16
On 29/04/2022 10:17, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 11/04/2022 12:32, Florian Weimer via Libc-alpha wrote:
>>> This is necessary to place the libio vtables into the RELRO segment.
>>> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
>>> verify that this is what actually happens.
>>>
>>> The new tests fail on ia64 due to lack of (default) RELRO support
>>> inbutils, so they are XFAILed there.
>>
>> I always frown upon new configure options, since they tend to degrade over
>> time (as --disable-shared for instance [1]) and require additional 
>> maintaining costs.  Why can't we make the required semantic the default 
>> (since usually relro has minimal costs it a net gain in hardening)?
> 
> This is what the patch does, I think.

I meant to not add --with-default-link= and just not use the extra linker 
script.
Florian Weimer April 29, 2022, 2:03 p.m. UTC | #17
* Adhemerval Zanella:

> On 29/04/2022 10:17, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 11/04/2022 12:32, Florian Weimer via Libc-alpha wrote:
>>>> This is necessary to place the libio vtables into the RELRO segment.
>>>> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
>>>> verify that this is what actually happens.
>>>>
>>>> The new tests fail on ia64 due to lack of (default) RELRO support
>>>> inbutils, so they are XFAILed there.
>>>
>>> I always frown upon new configure options, since they tend to degrade over
>>> time (as --disable-shared for instance [1]) and require additional 
>>> maintaining costs.  Why can't we make the required semantic the default 
>>> (since usually relro has minimal costs it a net gain in hardening)?
>> 
>> This is what the patch does, I think.
>
> I meant to not add --with-default-link= and just not use the extra linker 
> script.

That would be nice.  I want to get rid of the pointer arrays, and after
the vfprintf changes (and similar vscanf changes) are merged, we won't
need the vtable array anymore, either.  That should eliminate all
special section placement requirements.

Thanks,
Florian
Florian Weimer April 29, 2022, 2:55 p.m. UTC | #18
* Fangrui Song:

> On 2022-04-29, Florian Weimer wrote:
>>* Fangrui Song:
>>
>>>>How does lld place sections without a linker script?  Purely based on
>>>>names?
>>>
>>> It entirely uses code to describe the built-in rules like gold, while
>>> GNU ld uses an internal linker script plus built-in code.
>>> (some rules cannot be described by the linker script language and needs
>>> code anyway).
>>
>>How can we make sure that certain sections are covered by RELRO, and
>>still get start/stop symbols for them?
>
> About __start_/__stop_:
> __start_/__stop_ symbols are special. They don't need to be mentioned in a
> linker script to take effects for lld and modern GNU ld. I haven't checked
> whether there is an ancient GNU ld which requires symbol assignments when the
> output section description is specified.

According to the BFD ld documentation, __start_/__stop_ symbols are only
generated if the resulting name is a C identifier and the name is the
same.  That means that if we name the section .data.rel.ro.vtables or
something like that (to benefit from .data.rel.ro.* placement), we won't
get the start/stop symbols.  If we use a C identifier name for the
section, it won't be covered by RELRO.

> About RELRO:
> It seems that there is no section name convention to make a section
> RELRO.  Having a SECTIONS command with DATA_SEGMENT_ALIGN /
> DATA_SEGMENT_RELRO_END can specify all RELRO sections. I do not know a
> simpler approach than postprocessing ld.bfd --verbose output.
>
> To make GNU ld work, the following linker script is sufficient:
>
>   SECTIONS {
>            __libc_subfreeres : { *(__libc_subfreeres) }
>            __libc_atexit : { *(__libc_atexit) }
>            __libc_IO_vtables : { *(__libc_IO_vtables) }
>   } INSERT BEFORE .data.rel.ro;

That might work.  But if we cut down the number of vtables, I think we
can eliminate those special RELRO sections altogether.

We have been slowly migrating off __libc_subfreeres and the like because
having shutdown order depend on link is very non-obvious and
problematic.  For example, for optimimum results, we want to shut down
malloc last on a thread.

Thanks,
Florian
Dmitry V. Levin April 29, 2022, 4:02 p.m. UTC | #19
On Fri, Apr 22, 2022 at 08:35:37AM +0200, Florian Weimer via Libc-alpha wrote:
> * Siddhesh Poyarekar:
> 
> >> diff --git a/INSTALL b/INSTALL
> >> index 63c022d6b9..de150f66eb 100644
> >> --- a/INSTALL
> >> +++ b/INSTALL
> >> @@ -90,6 +90,12 @@ 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-default-link=FLAG'
> >> +     With '--with-default-link=yes', the GNU C Library does not use a
> >> +     custom linker scipt for linking its individual shared objects.  The
> >> +     default for FLAG is the opposite, 'no', because the custom linker
> >> +     script is needed for full RELRO protection.
> >> +
> >
> > Andreas' comments still apply here I think, i.e. fix the "scipt" type
> > and rephrase so that it's clearer that the option controls the library 
> > build process and not the library itself.
> 
> I thought I had fixed this.  What about this?
> 
> '--with-default-link=FLAG'
>      With '--with-default-link=yes', the build system does not use a
>      custom linker script for linking shared objects.  The default for
>      FLAG is the opposite, 'no', because the custom linker script is
>      needed for full RELRO protection.
> 
> > Does it make sense to make this --disable-custom-link or
> > --enable-default-link instead, since the option is binary?  The --with 
> > prefix is typically for richer options, e.g. paths.  Suggest something
> > like this:
> >
> > --disable-custom-link
> >     Don't use a custom linker script to build the GNU C Library,
> >     preferring the static linker's default script instead.  The custom
> >     linker script is needed for full RELRO protection.
> 
> I want to backport this, and distributions are already using this
> option, so I prefer not to make changes here.

I'm sorry to remind you something you know pretty well - the traditional
recommendations on choosing --enable-FEATURE[=ARG]/--disable-FEATURE[1]
vs --with-PACKAGE[=ARG]/--without-PACKAGE[2].

The first pair of options is to specify which build-time features to
enable, and the second one is to specify which external software to use.

If a downstream does not follow the documentation, this fact shouldn't be
normally used as a rationale for not following the documentation upstream.

If changes are accepted upstream as is just because they are already in
use downstream, this would open the way to all kinds of sloppy changes.

[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.71/html_node/Package-Options.html
[2] https://www.gnu.org/software/autoconf/manual/autoconf-2.71/html_node/External-Software.html
Florian Weimer April 29, 2022, 4:14 p.m. UTC | #20
* Dmitry V. Levin:

> On Fri, Apr 22, 2022 at 08:35:37AM +0200, Florian Weimer via Libc-alpha wrote:
>> * Siddhesh Poyarekar:
>> 
>> >> diff --git a/INSTALL b/INSTALL
>> >> index 63c022d6b9..de150f66eb 100644
>> >> --- a/INSTALL
>> >> +++ b/INSTALL
>> >> @@ -90,6 +90,12 @@ 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-default-link=FLAG'
>> >> +     With '--with-default-link=yes', the GNU C Library does not use a
>> >> +     custom linker scipt for linking its individual shared objects.  The
>> >> +     default for FLAG is the opposite, 'no', because the custom linker
>> >> +     script is needed for full RELRO protection.
>> >> +
>> >
>> > Andreas' comments still apply here I think, i.e. fix the "scipt" type
>> > and rephrase so that it's clearer that the option controls the library 
>> > build process and not the library itself.
>> 
>> I thought I had fixed this.  What about this?
>> 
>> '--with-default-link=FLAG'
>>      With '--with-default-link=yes', the build system does not use a
>>      custom linker script for linking shared objects.  The default for
>>      FLAG is the opposite, 'no', because the custom linker script is
>>      needed for full RELRO protection.
>> 
>> > Does it make sense to make this --disable-custom-link or
>> > --enable-default-link instead, since the option is binary?  The --with 
>> > prefix is typically for richer options, e.g. paths.  Suggest something
>> > like this:
>> >
>> > --disable-custom-link
>> >     Don't use a custom linker script to build the GNU C Library,
>> >     preferring the static linker's default script instead.  The custom
>> >     linker script is needed for full RELRO protection.
>> 
>> I want to backport this, and distributions are already using this
>> option, so I prefer not to make changes here.
>
> I'm sorry to remind you something you know pretty well - the traditional
> recommendations on choosing --enable-FEATURE[=ARG]/--disable-FEATURE[1]
> vs --with-PACKAGE[=ARG]/--without-PACKAGE[2].
>
> The first pair of options is to specify which build-time features to
> enable, and the second one is to specify which external software to use.
>
> If a downstream does not follow the documentation, this fact shouldn't be
> normally used as a rationale for not following the documentation upstream.
>
> If changes are accepted upstream as is just because they are already in
> use downstream, this would open the way to all kinds of sloppy changes.

I think your comment is based on an assumption that this is a new
option?

This change did not add the --with-default-link configure option, only
the documentation for it, and it changed the default to the safe one.

Thanks,
Florian
Fangrui Song April 29, 2022, 7:26 p.m. UTC | #21
On 2022-04-29, Florian Weimer wrote:
>* Fangrui Song:
>
>> On 2022-04-29, Florian Weimer wrote:
>>>* Fangrui Song:
>>>
>>>>>How does lld place sections without a linker script?  Purely based on
>>>>>names?
>>>>
>>>> It entirely uses code to describe the built-in rules like gold, while
>>>> GNU ld uses an internal linker script plus built-in code.
>>>> (some rules cannot be described by the linker script language and needs
>>>> code anyway).
>>>
>>>How can we make sure that certain sections are covered by RELRO, and
>>>still get start/stop symbols for them?
>>
>> About __start_/__stop_:
>> __start_/__stop_ symbols are special. They don't need to be mentioned in a
>> linker script to take effects for lld and modern GNU ld. I haven't checked
>> whether there is an ancient GNU ld which requires symbol assignments when the
>> output section description is specified.
>
>According to the BFD ld documentation, __start_/__stop_ symbols are only
>generated if the resulting name is a C identifier and the name is the
>same.  That means that if we name the section .data.rel.ro.vtables or
>something like that (to benefit from .data.rel.ro.* placement), we won't
>get the start/stop symbols.  If we use a C identifier name for the
>section, it won't be covered by RELRO.

I know the rule.  I meant I don't find a way making lld build have
RELRO __libc_* without the help of ld.bfd --verbose ;-)

>> About RELRO:
>> It seems that there is no section name convention to make a section
>> RELRO.  Having a SECTIONS command with DATA_SEGMENT_ALIGN /
>> DATA_SEGMENT_RELRO_END can specify all RELRO sections. I do not know a
>> simpler approach than postprocessing ld.bfd --verbose output.
>>
>> To make GNU ld work, the following linker script is sufficient:
>>
>>   SECTIONS {
>>            __libc_subfreeres : { *(__libc_subfreeres) }
>>            __libc_atexit : { *(__libc_atexit) }
>>            __libc_IO_vtables : { *(__libc_IO_vtables) }
>>   } INSERT BEFORE .data.rel.ro;
>
>That might work.  But if we cut down the number of vtables, I think we
>can eliminate those special RELRO sections altogether.
>
>We have been slowly migrating off __libc_subfreeres and the like because
>having shutdown order depend on link is very non-obvious and
>problematic.  For example, for optimimum results, we want to shut down
>malloc last on a thread.

If there is a way to get rid of __libc_* output sections, it will
certainly be great!

>Thanks,
>Florian
>
Dmitry V. Levin April 29, 2022, 8:48 p.m. UTC | #22
On Fri, Apr 29, 2022 at 06:14:27PM +0200, Florian Weimer wrote:
> * Dmitry V. Levin:
> 
> > On Fri, Apr 22, 2022 at 08:35:37AM +0200, Florian Weimer via Libc-alpha wrote:
> >> * Siddhesh Poyarekar:
> >> 
> >> >> diff --git a/INSTALL b/INSTALL
> >> >> index 63c022d6b9..de150f66eb 100644
> >> >> --- a/INSTALL
> >> >> +++ b/INSTALL
> >> >> @@ -90,6 +90,12 @@ 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-default-link=FLAG'
> >> >> +     With '--with-default-link=yes', the GNU C Library does not use a
> >> >> +     custom linker scipt for linking its individual shared objects.  The
> >> >> +     default for FLAG is the opposite, 'no', because the custom linker
> >> >> +     script is needed for full RELRO protection.
> >> >> +
> >> >
> >> > Andreas' comments still apply here I think, i.e. fix the "scipt" type
> >> > and rephrase so that it's clearer that the option controls the library 
> >> > build process and not the library itself.
> >> 
> >> I thought I had fixed this.  What about this?
> >> 
> >> '--with-default-link=FLAG'
> >>      With '--with-default-link=yes', the build system does not use a
> >>      custom linker script for linking shared objects.  The default for
> >>      FLAG is the opposite, 'no', because the custom linker script is
> >>      needed for full RELRO protection.
> >> 
> >> > Does it make sense to make this --disable-custom-link or
> >> > --enable-default-link instead, since the option is binary?  The --with 
> >> > prefix is typically for richer options, e.g. paths.  Suggest something
> >> > like this:
> >> >
> >> > --disable-custom-link
> >> >     Don't use a custom linker script to build the GNU C Library,
> >> >     preferring the static linker's default script instead.  The custom
> >> >     linker script is needed for full RELRO protection.
> >> 
> >> I want to backport this, and distributions are already using this
> >> option, so I prefer not to make changes here.
> >
> > I'm sorry to remind you something you know pretty well - the traditional
> > recommendations on choosing --enable-FEATURE[=ARG]/--disable-FEATURE[1]
> > vs --with-PACKAGE[=ARG]/--without-PACKAGE[2].
> >
> > The first pair of options is to specify which build-time features to
> > enable, and the second one is to specify which external software to use.
> >
> > If a downstream does not follow the documentation, this fact shouldn't be
> > normally used as a rationale for not following the documentation upstream.
> >
> > If changes are accepted upstream as is just because they are already in
> > use downstream, this would open the way to all kinds of sloppy changes.
> 
> I think your comment is based on an assumption that this is a new
> option?

Yes, the suggestion of new option --disable-custom-link somehow made me
think that --with-default-link is also a new option name, not something
that goes back to 2011.
diff mbox series

Patch

diff --git a/INSTALL b/INSTALL
index 63c022d6b9..de150f66eb 100644
--- a/INSTALL
+++ b/INSTALL
@@ -90,6 +90,12 @@  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-default-link=FLAG'
+     With '--with-default-link=yes', the GNU C Library does not use a
+     custom linker scipt for linking its individual shared objects.  The
+     default for FLAG is the opposite, 'no', because the custom linker
+     script is needed for full RELRO protection.
+
 '--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/configure b/configure
index d2f413d05d..650bfd982c 100755
--- a/configure
+++ b/configure
@@ -3375,7 +3375,7 @@  fi
 if test "${with_default_link+set}" = set; then :
   withval=$with_default_link; use_default_link=$withval
 else
-  use_default_link=default
+  use_default_link=no
 fi
 
 
@@ -6184,69 +6184,6 @@  fi
 $as_echo "$libc_cv_hashstyle" >&6; }
 
 
-# The linker's default -shared behavior is good enough if it
-# does these things that our custom linker scripts ensure that
-# all allocated NOTE sections come first.
-if test "$use_default_link" = default; then
-  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for sufficient default -shared layout" >&5
-$as_echo_n "checking for sufficient default -shared layout... " >&6; }
-if ${libc_cv_use_default_link+:} false; then :
-  $as_echo_n "(cached) " >&6
-else
-    libc_cv_use_default_link=no
-  cat > conftest.s <<\EOF
-	  .section .note.a,"a",%note
-	  .balign 4
-	  .long 4,4,9
-	  .string "GNU"
-	  .string "foo"
-	  .section .note.b,"a",%note
-	  .balign 4
-	  .long 4,4,9
-	  .string "GNU"
-	  .string "bar"
-EOF
-  if { ac_try='  ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&5'
-  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
-  (eval $ac_try) 2>&5
-  ac_status=$?
-  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
-  test $ac_status = 0; }; } &&
-       ac_try=`$READELF -S conftest.so | sed -n \
-	 '${x;p;}
-	  s/^ *\[ *[1-9][0-9]*\]  *\([^ ][^ ]*\)  *\([^ ][^ ]*\) .*$/\2 \1/
-	  t a
-	  b
-	  : a
-	  H'`
-  then
-    libc_seen_a=no libc_seen_b=no
-    set -- $ac_try
-    while test $# -ge 2 -a "$1" = NOTE; do
-      case "$2" in
-      .note.a) libc_seen_a=yes ;;
-      .note.b) libc_seen_b=yes ;;
-      esac
-      shift 2
-    done
-    case "$libc_seen_a$libc_seen_b" in
-    yesyes)
-      libc_cv_use_default_link=yes
-      ;;
-    *)
-      echo >&5 "\
-$libc_seen_a$libc_seen_b from:
-$ac_try"
-      ;;
-    esac
-  fi
-  rm -f conftest*
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_use_default_link" >&5
-$as_echo "$libc_cv_use_default_link" >&6; }
-  use_default_link=$libc_cv_use_default_link
-fi
-
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for GLOB_DAT reloc" >&5
 $as_echo_n "checking for GLOB_DAT reloc... " >&6; }
 if ${libc_cv_has_glob_dat+:} false; then :
diff --git a/configure.ac b/configure.ac
index b6a747dece..605efd549d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -153,7 +153,7 @@  AC_ARG_WITH([default-link],
 	    AS_HELP_STRING([--with-default-link],
 			   [do not use explicit linker scripts]),
 	    [use_default_link=$withval],
-	    [use_default_link=default])
+	    [use_default_link=no])
 
 dnl Additional build flags injection.
 AC_ARG_WITH([nonshared-cflags],
@@ -1371,59 +1371,6 @@  fi
 rm -f conftest*])
 AC_SUBST(libc_cv_hashstyle)
 
-# The linker's default -shared behavior is good enough if it
-# does these things that our custom linker scripts ensure that
-# all allocated NOTE sections come first.
-if test "$use_default_link" = default; then
-  AC_CACHE_CHECK([for sufficient default -shared layout],
-		  libc_cv_use_default_link, [dnl
-  libc_cv_use_default_link=no
-  cat > conftest.s <<\EOF
-	  .section .note.a,"a",%note
-	  .balign 4
-	  .long 4,4,9
-	  .string "GNU"
-	  .string "foo"
-	  .section .note.b,"a",%note
-	  .balign 4
-	  .long 4,4,9
-	  .string "GNU"
-	  .string "bar"
-EOF
-  if AC_TRY_COMMAND([dnl
-  ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&AS_MESSAGE_LOG_FD]) &&
-       ac_try=`$READELF -S conftest.so | sed -n \
-	 ['${x;p;}
-	  s/^ *\[ *[1-9][0-9]*\]  *\([^ ][^ ]*\)  *\([^ ][^ ]*\) .*$/\2 \1/
-	  t a
-	  b
-	  : a
-	  H']`
-  then
-    libc_seen_a=no libc_seen_b=no
-    set -- $ac_try
-    while test $# -ge 2 -a "$1" = NOTE; do
-      case "$2" in
-      .note.a) libc_seen_a=yes ;;
-      .note.b) libc_seen_b=yes ;;
-      esac
-      shift 2
-    done
-    case "$libc_seen_a$libc_seen_b" in
-    yesyes)
-      libc_cv_use_default_link=yes
-      ;;
-    *)
-      echo >&AS_MESSAGE_LOG_FD "\
-$libc_seen_a$libc_seen_b from:
-$ac_try"
-      ;;
-    esac
-  fi
-  rm -f conftest*])
-  use_default_link=$libc_cv_use_default_link
-fi
-
 AC_CACHE_CHECK(for GLOB_DAT reloc,
 	       libc_cv_has_glob_dat, [dnl
 cat > conftest.c <<EOF
diff --git a/elf/Makefile b/elf/Makefile
index c96924e9c2..366559fc96 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -543,6 +543,39 @@  endif
 endif
 endif
 
+tests-special += $(objpfx)tst-relro-ldso.out $(objpfx)tst-relro-libc.out
+$(objpfx)tst-relro-ldso.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
+  $(objpfx)ld.so
+	$(PYTHON) tst-relro-symbols.py $(objpfx)ld.so \
+	  --required=_rtld_global_ro \
+	  > $@ 2>&1; $(evaluate-test)
+# The optional symbols are present in libc only if the architecture has
+# the GLIBC_2.0 symbol set in libc.
+$(objpfx)tst-relro-libc.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
+  $(common-objpfx)libc.so
+	$(PYTHON) tst-relro-symbols.py $(common-objpfx)libc.so \
+	    --required=_IO_cookie_jumps \
+	    --required=_IO_file_jumps \
+	    --required=_IO_file_jumps_maybe_mmap \
+	    --required=_IO_file_jumps_mmap \
+	    --required=_IO_helper_jumps \
+	    --required=_IO_mem_jumps \
+	    --required=_IO_obstack_jumps \
+	    --required=_IO_proc_jumps \
+	    --required=_IO_str_chk_jumps \
+	    --required=_IO_str_jumps \
+	    --required=_IO_strn_jumps \
+	    --required=_IO_wfile_jumps \
+	    --required=_IO_wfile_jumps_maybe_mmap \
+	    --required=_IO_wfile_jumps_mmap \
+	    --required=_IO_wmem_jumps \
+	    --required=_IO_wstr_jumps \
+	    --required=_IO_wstrn_jumps \
+	    --optional=_IO_old_cookie_jumps \
+	    --optional=_IO_old_file_jumps \
+	    --optional=_IO_old_proc_jumps \
+	  > $@ 2>&1; $(evaluate-test)
+
 ifeq ($(run-built-tests),yes)
 tests-special += $(objpfx)tst-valgrind-smoke.out
 endif
diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
new file mode 100644
index 0000000000..368ea3349f
--- /dev/null
+++ b/elf/tst-relro-symbols.py
@@ -0,0 +1,137 @@ 
+#!/usr/bin/python3
+# Verify that certain symbols are covered by RELRO.
+# Copyright (C) 2022 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/>.
+
+"""Analyze a (shared) object to verify that certain symbols are
+present and covered by the PT_GNU_RELRO segment.
+
+"""
+
+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
+
+def find_relro(path: str, img: glibcelf.Image) -> (int, int):
+    """Discover the address range of the PT_GNU_RELRO segment."""
+    for phdr in img.phdrs():
+        if phdr.p_type == glibcelf.Pt.PT_GNU_RELRO:
+            # The computation is not entirely accurate because
+            # _dl_protect_relro in elf/dl-reloc.c rounds both the
+            # start end and downwards using the run-time page size.
+            return phdr.p_vaddr, phdr.p_vaddr + phdr.p_memsz
+    sys.stdout.write('{}: error: no PT_GNU_RELRO segment\n'.format(path))
+    sys.exit(1)
+
+def check_in_relro(kind, relro_begin, relro_end, name, start, size, error):
+    """Check if a section or symbol falls within in the RELRO segment."""
+    end = start + size - 1
+    if not (relro_begin <= start < end < relro_end):
+        error(
+            '{} {!r} of size {} at 0x{:x} is not in RELRO range [0x{:x}, 0x{:x})'.format(
+                kind, name.decode('UTF-8'), start, size,
+                relro_begin, relro_end))
+
+def get_parser():
+    """Return an argument parser for this script."""
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument('object', help='path to object file to check')
+    parser.add_argument('--required', metavar='NAME', default=(),
+                        help='required symbol names', nargs='*')
+    parser.add_argument('--optional', metavar='NAME', default=(),
+                        help='required symbol names', nargs='*')
+    return parser
+
+def main(argv):
+    """The main entry point."""
+    parser = get_parser()
+    opts = parser.parse_args(argv)
+    img = glibcelf.Image.readfile(opts.object)
+
+    required_symbols = frozenset([sym.encode('UTF-8')
+                                  for sym in opts.required])
+    optional_symbols = frozenset([sym.encode('UTF-8')
+                                  for sym in opts.optional])
+    check_symbols = required_symbols | optional_symbols
+
+    # Tracks the symbols in check_symbols that have been found.
+    symbols_found = set()
+
+    # Discover the extent of the RELRO segment.
+    relro_begin, relro_end = find_relro(opts.object, img)
+    symbol_table_found = False
+
+    errors = False
+    def error(msg: str) -> None:
+        """Record an error condition and write a message to standard output."""
+        nonlocal errors
+        errors = True
+        sys.stdout.write('{}: error: {}\n'.format(opts.object, msg))
+
+    # Iterate over section headers to find the symbol table.
+    for shdr in img.shdrs():
+        if shdr.sh_type == glibcelf.Sht.SHT_SYMTAB:
+            symbol_table_found = True
+            for sym in img.syms(shdr):
+                if sym.st_name in check_symbols:
+                    symbols_found.add(sym.st_name)
+
+                    # Validate symbol type, section, and size.
+                    if sym.st_info.type != glibcelf.Stt.STT_OBJECT:
+                        error('symbol {!r} has wrong type {}'.format(
+                            sym.st_name.decode('UTF-8'), sym.st_info.type))
+                    if sym.st_shndx in glibcelf.Shn:
+                        error('symbol {!r} has reserved section {}'.format(
+                            sym.st_name.decode('UTF-8'), sym.st_shndx))
+                        continue
+                    if sym.st_size == 0:
+                        error('symbol {!r} has size zero'.format(
+                            sym.st_name.decode('UTF-8')))
+                        continue
+
+                    check_in_relro('symbol', relro_begin, relro_end,
+                                   sym.st_name, sym.st_value, sym.st_size,
+                                   error)
+            continue # SHT_SYMTAB
+        if shdr.sh_name == b'.data.rel.ro' \
+           or shdr.sh_name.startswith(b'.data.rel.ro.'):
+            check_in_relro('section', relro_begin, relro_end,
+                           shdr.sh_name, shdr.sh_addr, shdr.sh_size,
+                           error)
+            continue
+
+    if required_symbols - symbols_found:
+        for sym in sorted(required_symbols - symbols_found):
+            error('symbol {!r} not found'.format(sym.decode('UTF-8')))
+
+    if errors:
+        sys.exit(1)
+
+    if not symbol_table_found:
+        sys.stdout.write(
+            '{}: warning: no symbol table found (stripped object)\n'.format(
+                opts.object))
+        sys.exit(77)
+
+if __name__ == '__main__':
+    main(sys.argv[1:])
diff --git a/manual/install.texi b/manual/install.texi
index 29c52f2927..fcfb6901e4 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -117,6 +117,12 @@  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-default-link=@var{FLAG}
+With @code{--with-default-link=yes}, the build system does not use a
+custom linker script for linking shared objects.  The default for
+@var{FLAG} is the opposite, @samp{no}, because the custom linker script
+is needed for full RELRO protection.
+
 @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
diff --git a/sysdeps/unix/sysv/linux/ia64/Makefile b/sysdeps/unix/sysv/linux/ia64/Makefile
index da85ba43e2..c5cc41b367 100644
--- a/sysdeps/unix/sysv/linux/ia64/Makefile
+++ b/sysdeps/unix/sysv/linux/ia64/Makefile
@@ -1,3 +1,9 @@ 
+ifeq ($(subdir),elf)
+# ia64 does not support PT_GNU_RELRO.
+test-xfail-tst-relro-ldso = yes
+test-xfail-tst-relro-libc = yes
+endif
+
 ifeq ($(subdir),misc)
 sysdep_headers += sys/rse.h
 endif