From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb2b.google.com (mail-yb1-xb2b.google.com [IPv6:2607:f8b0:4864:20::b2b]) by sourceware.org (Postfix) with ESMTPS id CD0C43858C27 for ; Mon, 18 Oct 2021 20:11:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CD0C43858C27 Received: by mail-yb1-xb2b.google.com with SMTP id i84so467104ybc.12 for ; Mon, 18 Oct 2021 13:11:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=MwJR+ZYc7YUtX1tGi0IqyRgr+Uv+adaAwRe6uuEL9xk=; b=qO+zE4MNVdY44in4dBHCx+I7BE1q64BTYm9BvEc+jcgQ3TW23M5uVJoWDLgrJZ6XYT cQfOLN4pV27kkYt1pGNe2lRwAf/QhLfsAMLacn6apIE8rxtjaeF3ANyAGDbQand48A7E qI5in96RnqcraaBOs3SG4uMxln3U75Xg3nq0GCUMETUj/hbosxbmCfnDz/zW98Oyc5uq AtMH6y1Dg/Hd4147OI/79IxlvkayKVzqpM29KqcYGN4MwWZFDyIYeeuUpLthMjgbIKGE 9pYzLNw3pIjkPmOg5b12LVafwgOmsUKl3WpVf+O3V5gPo/uZyJUGIN4BFPnT2uK5ndlQ Gjdg== X-Gm-Message-State: AOAM531oqZHpGBTLQ04Ysq+MfEyFCqZxSIAIp35M1d0nKtmruGPgqp/I /yVHrSAEgGkxxmak/GSf5jKL5lCyY6IGFgQndsAjrg== X-Google-Smtp-Source: ABdhPJxm4w71VHyUR9pI00IjTwOoGxSnkWgMy15YURXheTYVhhVMpw6mFvHSoRE+/eM8yahtLeBc4J+fwNh7JXakHqs= X-Received: by 2002:a25:bcc:: with SMTP id 195mr15177216ybl.492.1634587878177; Mon, 18 Oct 2021 13:11:18 -0700 (PDT) MIME-Version: 1.0 References: <20211017005020.2645717-1-maskray@google.com> <20211018181530.yz62n64dgn2kd4oe@google.com> <20211018191902.vcf3u5s7h5losxyt@google.com> In-Reply-To: From: =?UTF-8?B?RsSBbmctcnXDrCBTw7JuZw==?= Date: Mon, 18 Oct 2021 13:11:06 -0700 Message-ID: Subject: Re: [PATCH v2] elf: Support DT_RELR relative relocation format [BZ #27924] To: "H.J. Lu" Cc: GNU C Library , Binutils Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-23.6 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL 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: Mon, 18 Oct 2021 20:11:23 -0000 On Mon, Oct 18, 2021 at 12:45 PM H.J. Lu wrote: > > On Mon, Oct 18, 2021 at 12:19 PM F=C4=81ng-ru=C3=AC S=C3=B2ng wrote: > > > > On 2021-10-18, H.J. Lu wrote: > > >On Mon, Oct 18, 2021 at 11:15 AM F=C4=81ng-ru=C3=AC S=C3=B2ng wrote: > > >> > > >> > > >> On 2021-10-18, H.J. Lu wrote: > > >> >On Mon, Oct 18, 2021 at 9:17 AM F=C4=81ng-ru=C3=AC S=C3=B2ng wrote: > > >> >> > > >> >> On Mon, Oct 18, 2021 at 7:42 AM H.J. Lu wro= te: > > >> >> > > > >> >> > On Sat, Oct 16, 2021 at 5:50 PM Fangrui Song via Binutils > > >> >> > wrote: > > >> >> > > > > >> >> > > PIE and shared objects usually have many relative relocations= . In > > >> >> > > 2017/2018, SHT_RELR/DT_RELR was proposed on > > >> >> > > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0= L-PBAAJ > > >> >> > > ("Proposal for a new section type SHT_RELR") and is a pre-sta= ndard. RELR > > >> >> > > usually takes 3% or smaller space than R_*_RELATIVE relocatio= ns. The > > >> >> > > virtual memory size of a mostly statically linked PIE is typi= cally 5~10% > > >> >> > > smaller. > > >> >> > > > > >> >> > > --- > > >> >> > > > > >> >> > > Notes I will not include in the submitted commit: > > >> >> > > > > >> >> > > Available on https://sourceware.org/git/?p=3Dglibc.git;a=3Dsh= ortlog;h=3Drefs/heads/maskray/relr > > >> >> > > > > >> >> > > "pre-standard": even Solaris folks are happy with the refined= generic-abi > > >> >> > > proposal. Cary Coutant will apply the change > > >> >> > > https://sourceware.org/pipermail/libc-alpha/2021-October/1317= 81.html > > >> >> > > > > >> >> > > This patch is simpler than Chrome OS's glibc patch and makes = ELF_DYNAMIC_DO_RELR > > >> >> > > available to all ports. I don't think the current glibc imple= mentation > > >> >> > > supports ia64 in an ELFCLASS32 container. That said, the styl= e I used is > > >> >> > > works with an ELFCLASS32 container for 64-bit machine if ElfW= (Addr) is > > >> >> > > 64-bit. > > >> >> > > > > >> >> > > * Chrome OS folks have carried a local patch since 2018 (late= st version: > > >> >> > > https://chromium.googlesource.com/chromiumos/overlays/chrom= iumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32). > > >> >> > > I.e. this feature has been battle tested. > > >> >> > > * Android bionic supports 2018 and switched to DT_RELR=3D=3D3= 6 in 2020. > > >> >> > > * The Linux kernel has supported CONFIG_RELR since 2019-08 > > >> >> > > (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213= e27be8bdb1d). > > >> >> > > * A musl patch (by me) exists but is not applied: > > >> >> > > https://www.openwall.com/lists/musl/2019/03/06/3 > > >> >> > > * rtld-elf from FreeBSD 14 will support DT_RELR. > > >> >> > > > > >> >> > > I believe upstream glibc should support DT_RELR to benefit al= l Linux > > >> >> > > distributions. I filed some feature requests to get their att= ention: > > >> >> > > > > >> >> > > * Gentoo: https://bugs.gentoo.org/818376 > > >> >> > > * Arch Linux: https://bugs.archlinux.org/task/72433 > > >> >> > > * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=3D= 996598 > > >> >> > > * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=3D201469= 9 > > >> >> > > > > >> >> > > As of linker support (to the best of my knowledge): > > >> >> > > > > >> >> > > * LLD support DT_RELR. > > >> >> > > * https://chromium.googlesource.com/chromiumos/overlays/chrom= iumos-overlay/+/refs/heads/main/sys-devel/binutils/files/ > > >> >> > > has a gold patch. > > >> >> > > * GNU ld feature request https://sourceware.org/bugzilla/show= _bug.cgi?id=3D27923 > > >> >> > > > > >> >> > > I wish that GNU ld and gold maintainers can implement the fea= ture as well :) > > >> >> > > > > >> >> > > Tested on aarch64 and x86_64. > > >> >> > > > > >> >> > > Changes from v1 (https://sourceware.org/pipermail/libc-alpha/= 2021-October/131768.html) > > >> >> > > * Fix style, simplify code > > >> >> > > * Improve test > > >> >> > > --- > > >> >> > > configure | 31 +++++++++++++++++++++++++++++++ > > >> >> > > configure.ac | 4 ++++ > > >> >> > > elf/Makefile | 4 ++++ > > >> >> > > elf/dynamic-link.h | 28 ++++++++++++++++++++++++++++ > > >> >> > > elf/elf.h | 13 +++++++++++-- > > >> >> > > elf/get-dynamic-info.h | 3 +++ > > >> >> > > elf/tst-relr.c | 27 +++++++++++++++++++++++++++ > > >> >> > > 7 files changed, 108 insertions(+), 2 deletions(-) > > >> >> > > create mode 100644 elf/tst-relr.c > > >> >> > > > > >> >> > > diff --git a/configure b/configure > > >> >> > > index 3227e434d3..fdab6a97ef 100755 > > >> >> > > --- a/configure > > >> >> > > +++ b/configure > > >> >> > > @@ -6067,6 +6067,37 @@ $as_echo "$libc_linker_feature" >&6; } > > >> >> > > config_vars=3D"$config_vars > > >> >> > > have-depaudit =3D $libc_cv_depaudit" > > >> >> > > > > >> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker= that supports --pack-dyn-relocs=3Drelr" >&5 > > >> >> > > +$as_echo_n "checking for linker that supports --pack-dyn-rel= ocs=3Drelr... " >&6; } > > >> >> > > +libc_linker_feature=3Dno > > >> >> > > +if test x"$gnu_ld" =3D x"yes"; then > > >> >> > > + cat > conftest.c < > >> >> > > +int _start (void) { return 42; } > > >> >> > > +EOF > > >> >> > > + if { ac_try=3D'${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp > > >> >> > > + -Wl,--pack-dyn-relocs=3Drelr -nostdlib -n= ostartfiles > > >> >> > > + -fPIC -shared -o conftest.so conftest.c > > >> >> > > + 1>&5' > > >> >> > > + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try= \""; } >&5 > > >> >> > > + (eval $ac_try) 2>&5 > > >> >> > > + ac_status=3D$? > > >> >> > > + $as_echo "$as_me:${as_lineno-$LINENO}: \$? =3D $ac_status"= >&5 > > >> >> > > + test $ac_status =3D 0; }; } > > >> >> > > + then > > >> >> > > + libc_linker_feature=3Dyes > > >> >> > > + fi > > >> >> > > + rm -f conftest* > > >> >> > > +fi > > >> >> > > +if test $libc_linker_feature =3D yes; then > > >> >> > > + libc_cv_relr=3Dyes > > >> >> > > +else > > >> >> > > + libc_cv_relr=3Dno > > >> >> > > +fi > > >> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linke= r_feature" >&5 > > >> >> > > +$as_echo "$libc_linker_feature" >&6; } > > >> >> > > +config_vars=3D"$config_vars > > >> >> > > +have-relr =3D $libc_cv_relr" > > >> >> > > + > > >> >> > > { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker= that supports --no-dynamic-linker" >&5 > > >> >> > > $as_echo_n "checking for linker that supports --no-dynamic-l= inker... " >&6; } > > >> >> > > libc_linker_feature=3Dno > > >> >> > > diff --git a/configure.ac b/configure.ac > > >> >> > > index 00f49f09f7..96110f9d7d 100644 > > >> >> > > --- a/configure.ac > > >> >> > > +++ b/configure.ac > > >> >> > > @@ -1354,6 +1354,10 @@ LIBC_LINKER_FEATURE([--depaudit], [-Wl= ,--depaudit,x], > > >> >> > > [libc_cv_depaudit=3Dyes], [libc_cv_depaud= it=3Dno]) > > >> >> > > LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit]) > > >> >> > > > > >> >> > > +LIBC_LINKER_FEATURE([--pack-dyn-relocs=3Drelr], [-Wl,--pack-= dyn-relocs=3Drelr], > > >> >> > > + [libc_cv_relr=3Dyes], [libc_cv_relr=3Dno]= ) > > >> >> > > +LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr]) > > >> >> > > + > > >> >> > > LIBC_LINKER_FEATURE([--no-dynamic-linker], > > >> >> > > [-Wl,--no-dynamic-linker], > > >> >> > > [libc_cv_no_dynamic_linker=3Dyes], > > >> >> > > diff --git a/elf/Makefile b/elf/Makefile > > >> >> > > index bf45d8ee24..2c4cdfac68 100644 > > >> >> > > --- a/elf/Makefile > > >> >> > > +++ b/elf/Makefile > > >> >> > > @@ -245,6 +245,10 @@ tests-special +=3D $(objpfx)tst-audit14-= cmp.out $(objpfx)tst-audit15-cmp.out \ > > >> >> > > $(objpfx)tst-audit16-cmp.out > > >> >> > > endif > > >> >> > > endif > > >> >> > > +ifeq ($(have-relr),yes) > > >> >> > > +tests +=3D tst-relr > > >> >> > > +LDFLAGS-tst-relr +=3D -Wl,--pack-dyn-relocs=3Drelr > > >> >> > > +endif > > >> >> > > endif > > >> >> > > > >> >> > Is DT_RELR only generated for PIE? If yes, you need to add it > > >> >> > to tests-pie and compile it as PIE. > > >> >> > > >> >> PIE and shared objects. PDE doesn't need relative relocations. > > >> >> It is useful to ensure that -Wl,--pack-dyn-relocs=3Drelr doesn't = cause > > >> >> breakage to PDE. > > >> > > > >> >Then you need to add a PIE test to verify that > > >> > > > >> >1. It has DT_RELR. > > >> >2. It runs correctly. > > >> > > >> --- a/elf/Makefile > > >> +++ b/elf/Makefile > > >> @@ -246,4 +246,8 @@ tests-special +=3D $(objpfx)tst-audit14-cmp.out = $(objpfx)tst-audit15-cmp.out \ > > >> endif > > >> endif > > >> +ifeq ($(have-relr),yes) > > >> +tests +=3D tst-relr > > >> +LDFLAGS-tst-relr +=3D -Wl,--pack-dyn-relocs=3Drelr > > >> +endif > > >> endif > > >> tests +=3D $(tests-execstack-$(have-z-execstack)) > > >> > > >> I added this chunk to my test (https://sourceware.org/git/?p=3Dglibc= .git;a=3Dshortlog;h=3Drefs/heads/maskray/relr): > > >> > > >> ElfW(Dyn) *d =3D _DYNAMIC; > > >> if (d) > > >> { > > >> bool has_relr =3D false; > > >> for (; d->d_tag !=3D DT_NULL; d++) > > >> if (d->d_tag =3D=3D DT_RELR) > > >> has_relr =3D true; > > >> if (!has_relr) > > >> { > > >> fprintf (stderr, "no DT_RELR\n"); > > >> return 1; > > >> } > > >> } > > > > > >We need 2 tests from the same source: > > > > > >1. The DT_RELR linker option doesn't break PDE. > > >2. The DT_RELR linker option works for PIE. > > > > > >You only have one test. > > > > Thanks for the feedback. > > https://sourceware.org/git/?p=3Dglibc.git;a=3Dshortlog;h=3Drefs/heads/m= askray/relr > > has switched to > > > > --- a/elf/Makefile > > +++ b/elf/Makefile > > @@ -246,4 +246,10 @@ tests-special +=3D $(objpfx)tst-audit14-cmp.out $(= objpfx)tst-audit15-cmp.out \ > > endif > > endif > > +ifeq ($(have-relr),yes) > > +tests +=3D tst-relr tst-relr-no-pie > > You need one test with explicit PIE and one with explicit non-PIE > since not all compilers default to PIE. OK. I added: tests-pie +=3D tst-relr > > +tests-no-pie +=3D tst-relr-no-pie > > +LDFLAGS-tst-relr +=3D -Wl,--pack-dyn-relocs=3Drelr > > +LDFLAGS-tst-relr-no-pie +=3D -Wl,--pack-dyn-relocs=3Drelr > > +endif > > endif > > tests +=3D $(tests-execstack-$(have-z-execstack)) > > >> >> > [hjl@gnu-cfl-2 tmp]$ gcc -pie -fPIE -O2 tst-relr.c > > >> >> > -Wl,--pack-dyn-relocs=3Drelr -fuse-ld=3Dlld > > >> >> > [hjl@gnu-cfl-2 tmp]$ ./a.out > > >> >> > Segmentation fault (core dumped) > > >> >> > [hjl@gnu-cfl-2 tmp]$ > > >> >> > > > >> >> > Given that the current lld implementation generates broken > > >> >> > binaries for existing glibc without any warning at run-time, > > >> >> > we should use a different linker command line option to > > >> >> > implement it properly so that the new binary will fail to > > >> >> > run on glibc without DT_RELR support at run-time. > > >> >> > > >> >> I don't think so. LLD's design is to be machine agnostic and NOT = make > > >> >> decisions which would vary on different machines. > > >> >> --pack-dyn-relocs=3Drelr is not the default. The user tells LLD t= o use > > >> >> DT_RELR and the user is responsible for making sure target ld.so > > >> >> supports DT_RELR. > > >> >> LLD is often used as a cross linker. The host ld.so doesn't suppo= rt > > >> >> LLD doesn't mean that LLD should disable the format. > > >> >> For example, it is totally fine to link a FreeBSD executable on a= Linux machine. > > >> > > > >> >For Linux targets, we need the glibc version dependency for > > >> >DT_RELR support at run-time. > > >> > > >> I know that having an executable crash right away (SIGSEGV, if ld.so= doesn't > > >> resolve relative relocations) without a diagnostic is arguably poor = user experience, > > >> and you might come from this angle and therefore suggest a version c= heck. > > > > > >Correct. We need to resolve this before DT_RELR can be accepted. > > > > Introducing DT_RELR to provide an alternative way encoding relative rel= ocations > > is like introducing DT_INIT_ARRAY/DT_PREINIT_ARRAY to replace legacy .c= tors/.dtors , > > but more peaceful: it is a pure linker decision, not mixed compiler cod= egen and linker decision. > > > > In some sense, the problem is not too different from linker creating a = WX > > PT_LOAD segment and the system is configured with rejecting WX. The use= r > > provides input and options to make the linker do so. When something is = reasonably within > > the linker's error checking scope, an error checker can be added. But s= ome use cases > > have valid use of WX PT_LOAD (--omagic), why is the linker in the busin= ess of rejecting > > valid usage? > > > > This is not a glibc problem. We could add an error now in the absence o= f DT_RELR support. > > But that is moot when the support is coming. > > > > The problem is running the DT_RELR binary on the old ld.so, which > can't be changed. > > H.J. I know. I have mentioned that the situation is not different from many previous situations where a compatibility check may not catch everything. (When ET_DYN executable was first introduced, did it need to bump e_version or add something to e_flags? When DT_INIT_ARRAY was added? ... ) I did highlight that even in the absence of the check, the harm is minimal: --pack-dyn-relocs=3Drelr with new glibc will introduce new version requirement, which will be naturally caught by older ld.so. There are a ton non-default linker options which can create non-runnable binaries. In the end, I consider that the discussion is useful but has derailed from glibc getting the feature. Changes are made to new glibc, not old glibc. We cannot time travel to the past and change glibc to reject new features today. It is still valuable and we may need it again when GCC/distributions want to make relr the default for some targets.