From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bumble.maple.relay.mailchannels.net (bumble.maple.relay.mailchannels.net [23.83.214.25]) by sourceware.org (Postfix) with ESMTPS id 6DE573858C2C for ; Fri, 22 Apr 2022 06:12:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6DE573858C2C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 056CD2C0FA9; Fri, 22 Apr 2022 06:12:58 +0000 (UTC) Received: from pdx1-sub0-mail-a307.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id A52322C0B81; Fri, 22 Apr 2022 06:12:57 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1650607977; a=rsa-sha256; cv=none; b=3Q/fj4yfU38a4IUxMcrX6PdgPqh7Ey3GeGi8CHdKb0yr4FHHQ6qHHyhYoyZ0pq7sxWJjho LeHsWUTfog2Z8U7/uIk2TiTOt/C+N1zB3Ue7slx/K3xuIJirKGMe+wDIdqHbUOItIREEXV loOr9PJXoU8PNDNDirbYVAZxNlxcF7Pg+JEbdeinMUxp37JTVXeuMtc1GJCF4F7aFT1Ua3 fHc9Qy0Uq/dksDSB1FnhWwV7cH9Rm7ZU3KNCkZ3Gjs5JH8ZpoND/r54DULtibXCfL4BrBZ aEu/DFGYBDkHq6SlIu6YKZ9fs/S4xFyjwWxpDtU/rGu5XuAvYIvr4AMJLilbuQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1650607977; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=3bjWD1rrFLuu/HuioQVm7cKjVeaX102F6trCDHsfRxI=; b=wu1kXSzVYGR18t9IwSUt2wyGBf/Yrb96utdbxoFHPQu1PSp5K+cLDf5Oh5GYE2o+iLUz2o 5hNPYudBXykOkqHhzIIMvbezMTWm+sMA1UH+owxC7h0pUv7h7hJe3hyX7JQp6rErg+fm0W WH7cFjTUl0iv+SAP+FtGVe3RlGDnjeNqY35Vcc7DfvWOoIA4SCClX6PHTfnsepMmTh4zE5 DmPRtYACJozECJCqfcDwt/T8846UbB+WX1LYox/knbjvqFjt+QqaeFv9xPnEeD2Oj27kpY hPg57YLIf2cwt5JUmcasVuxnvxg87ey3qfVcjqoVlwDPMC3ykJTa5w8y8L/VEw== ARC-Authentication-Results: i=1; rspamd-7968956b8c-cc2ls; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@gotplt.org X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Coil-Share: 31f290ea43eb31d2_1650607977773_2487898152 X-MC-Loop-Signature: 1650607977773:3024967602 X-MC-Ingress-Time: 1650607977773 Received: from pdx1-sub0-mail-a307.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.96.96.20 (trex/6.7.1); Fri, 22 Apr 2022 06:12:57 +0000 Received: from [192.168.1.174] (unknown [1.186.121.46]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a307.dreamhost.com (Postfix) with ESMTPSA id 4Kl3x40XStzLC; Thu, 21 Apr 2022 23:12:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1650607977; bh=dm4BxyUWI71gK5qcUsPeBCb6lgmJD0b3ITnuzgHWlLw=; h=Date:Subject:To:From:Content-Type:Content-Transfer-Encoding; b=nlIK96uQ0XZ4vsUpx/oIbnq9oBrqDiKkbEowL6VZ9B0Hm4tAOuwENL2hakhGExDPE 0IlEt+0GvxZPcNzo2VGNTy9zfWjCOchjb7dD/ENJ/Fn9ZOC8NSkArL7Z/Z5F4mNMpc 8HM/ebAU6Aef8kKefH4FdX8xlJeuZE+WtoiBAyWNCdI9CbjeUj08o6QzAKnL3espR/ 4fIoUAsq8nz2QN6dL852Ow4f/cdxbf2TQIyw4Tm4VBPRk9tbe1RKbytNjko5CcwDco WPRvuRiHsEtZ29JrN+xCgFxh1EyIA9uYtCRg8U9fo5zfbyEzvEDFFm0VxYVs9Soqhr OJ4lhyHELAsKQ== Message-ID: Date: Fri, 22 Apr 2022 11:42:51 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812) Content-Language: en-US To: Florian Weimer , libc-alpha@sourceware.org References: <15c7f6e9afe2c2b9c51ebc6372682a39b0932712.1649691083.git.fweimer@redhat.com> From: Siddhesh Poyarekar In-Reply-To: <15c7f6e9afe2c2b9c51ebc6372682a39b0932712.1649691083.git.fweimer@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3039.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_SBL, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Apr 2022 06:13:04 -0000 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 < 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 > +# . > + > +"""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.