From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42a.google.com (mail-pf1-x42a.google.com [IPv6:2607:f8b0:4864:20::42a]) by sourceware.org (Postfix) with ESMTPS id E65FD3858C27; Mon, 18 Oct 2021 19:45:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E65FD3858C27 Received: by mail-pf1-x42a.google.com with SMTP id 187so15544124pfc.10; Mon, 18 Oct 2021 12:45:07 -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=pRPvhawJRdUpPLPAEmbL8q7V0YNZuSPz83WQYRIhpuc=; b=1A2pryv0p2Hq1lnsn46ln3MmMcyHtIvDjRnL8cod3tiLgJjPi8b0/QsKb7nEER9Nbz /s5wGmSFUHtQgWmGtzi08du1VIWqfnjLs6z/pqwTL5iPlnu7OFbjuN69d7B7Hlpl/0mT Ss2lnQE6IDiOVFsLGasTWfgjgE2CVA+1mKCi7Q6IxrV2GQ78XDZZPVm25kRu5nke3vNe tWY9G3xSufElvsvnQ/REEx2k6IZqnyUHyMkMCkfghKwxgRA4Ie9fbCPNA9s5J+Bsagk3 RvdHcVh4QhoP2vTTvZyXY06Dmceeyy7HxhELdpNKvptC66aN4ydia+SyExWw4Z1GyQbV ISBg== X-Gm-Message-State: AOAM530YbkXQNaQdkVo2sraYg5hYIwdJhtdo343KBjqycWYmGtj2CHvI mRESzsoIuvJckI0ax7z5dLLKjtGj5TsM5xWGWFxCWjh3 X-Google-Smtp-Source: ABdhPJx7i67RcVewsWjUybkkG995U2Ca3S+D8dpIo14XeODggOpXy3PMf1xJkvO0etSOoj2EMQjTSMdS1gmf4/5ysFY= X-Received: by 2002:a05:6a00:b95:b0:44c:7c8b:f762 with SMTP id g21-20020a056a000b9500b0044c7c8bf762mr31236982pfj.60.1634586306917; Mon, 18 Oct 2021 12:45:06 -0700 (PDT) MIME-Version: 1.0 References: <20211017005020.2645717-1-maskray@google.com> <20211018181530.yz62n64dgn2kd4oe@google.com> <20211018191902.vcf3u5s7h5losxyt@google.com> In-Reply-To: <20211018191902.vcf3u5s7h5losxyt@google.com> From: "H.J. Lu" Date: Mon, 18 Oct 2021 12:44:30 -0700 Message-ID: Subject: Re: [PATCH v2] elf: Support DT_RELR relative relocation format [BZ #27924] To: =?UTF-8?B?RsSBbmctcnXDrCBTw7JuZw==?= Cc: GNU C Library , Binutils Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-3029.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP 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 19:45:10 -0000 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 wrote= : > >> >> > > >> >> > 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/GxjM0L-= PBAAJ > >> >> > > ("Proposal for a new section type SHT_RELR") and is a pre-stand= ard. RELR > >> >> > > usually takes 3% or smaller space than R_*_RELATIVE relocations= . The > >> >> > > virtual memory size of a mostly statically linked PIE is typica= lly 5~10% > >> >> > > smaller. > >> >> > > > >> >> > > --- > >> >> > > > >> >> > > Notes I will not include in the submitted commit: > >> >> > > > >> >> > > Available on https://sourceware.org/git/?p=3Dglibc.git;a=3Dshor= tlog;h=3Drefs/heads/maskray/relr > >> >> > > > >> >> > > "pre-standard": even Solaris folks are happy with the refined g= eneric-abi > >> >> > > proposal. Cary Coutant will apply the change > >> >> > > https://sourceware.org/pipermail/libc-alpha/2021-October/131781= .html > >> >> > > > >> >> > > This patch is simpler than Chrome OS's glibc patch and makes EL= F_DYNAMIC_DO_RELR > >> >> > > available to all ports. I don't think the current glibc impleme= ntation > >> >> > > supports ia64 in an ELFCLASS32 container. That said, the style = I used is > >> >> > > works with an ELFCLASS32 container for 64-bit machine if ElfW(A= ddr) is > >> >> > > 64-bit. > >> >> > > > >> >> > > * Chrome OS folks have carried a local patch since 2018 (latest= version: > >> >> > > https://chromium.googlesource.com/chromiumos/overlays/chromiu= mos-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=3D36 = in 2020. > >> >> > > * The Linux kernel has supported CONFIG_RELR since 2019-08 > >> >> > > (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e2= 7be8bdb1d). > >> >> > > * 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 all = Linux > >> >> > > distributions. I filed some feature requests to get their atten= tion: > >> >> > > > >> >> > > * Gentoo: https://bugs.gentoo.org/818376 > >> >> > > * Arch Linux: https://bugs.archlinux.org/task/72433 > >> >> > > * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=3D99= 6598 > >> >> > > * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=3D2014699 > >> >> > > > >> >> > > As of linker support (to the best of my knowledge): > >> >> > > > >> >> > > * LLD support DT_RELR. > >> >> > > * https://chromium.googlesource.com/chromiumos/overlays/chromiu= mos-overlay/+/refs/heads/main/sys-devel/binutils/files/ > >> >> > > has a gold patch. > >> >> > > * GNU ld feature request https://sourceware.org/bugzilla/show_b= ug.cgi?id=3D27923 > >> >> > > > >> >> > > I wish that GNU ld and gold maintainers can implement the featu= re as well :) > >> >> > > > >> >> > > Tested on aarch64 and x86_64. > >> >> > > > >> >> > > Changes from v1 (https://sourceware.org/pipermail/libc-alpha/20= 21-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 t= hat supports --pack-dyn-relocs=3Drelr" >&5 > >> >> > > +$as_echo_n "checking for linker that supports --pack-dyn-reloc= s=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 -nos= tartfiles > >> >> > > + -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_linker_= 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 t= hat supports --no-dynamic-linker" >&5 > >> >> > > $as_echo_n "checking for linker that supports --no-dynamic-lin= ker... " >&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_depaudit= =3Dno]) > >> >> > > LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit]) > >> >> > > > >> >> > > +LIBC_LINKER_FEATURE([--pack-dyn-relocs=3Drelr], [-Wl,--pack-dy= n-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-cm= p.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 ca= use > >> >> 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.g= it;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/mas= kray/relr > has switched to > > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -246,4 +246,10 @@ tests-special +=3D $(objpfx)tst-audit14-cmp.out $(ob= jpfx)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. > +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 ma= ke > >> >> decisions which would vary on different machines. > >> >> --pack-dyn-relocs=3Drelr is not the default. The user tells LLD to = 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 support > >> >> LLD doesn't mean that LLD should disable the format. > >> >> For example, it is totally fine to link a FreeBSD executable on a L= inux 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 d= oesn't > >> resolve relative relocations) without a diagnostic is arguably poor us= er experience, > >> and you might come from this angle and therefore suggest a version che= ck. > > > >Correct. We need to resolve this before DT_RELR can be accepted. > > Introducing DT_RELR to provide an alternative way encoding relative reloc= ations > is like introducing DT_INIT_ARRAY/DT_PREINIT_ARRAY to replace legacy .cto= rs/.dtors , > but more peaceful: it is a pure linker decision, not mixed compiler codeg= en 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 user > provides input and options to make the linker do so. When something is re= asonably within > the linker's error checking scope, an error checker can be added. But som= e use cases > have valid use of WX PT_LOAD (--omagic), why is the linker in the busines= s of rejecting > valid usage? > > This is not a glibc problem. We could add an error now in the absence of = 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.