From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22c.google.com (mail-oi1-x22c.google.com [IPv6:2607:f8b0:4864:20::22c]) by sourceware.org (Postfix) with ESMTPS id 1F1BF3948491 for ; Thu, 14 Jan 2021 16:02:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1F1BF3948491 Received: by mail-oi1-x22c.google.com with SMTP id 9so6407110oiq.3 for ; Thu, 14 Jan 2021 08:02:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rLjx9m57HdXdFrs0fAoxF+ILgoM+1W5o1DikCGijLaM=; b=ZOnTxqdqPC1RlPh+FcBvfsfTcYbOgrf2Z0tfxs3D4jn3EQctQcSsEciHfKR6oxlJ48 /nO94jdZj/G2XFJoXI/j6z9aAIDmjCWEvmiixCPLWPpOYN/czjiB+QbJWj2iNAL0B4AF g+oyuFTJoEpaTWOLAD8Z2r4E+6pNw61jIOJ+m/G9WTDvw9tsQY6hiTMo7wxpjF8JN8tM 9Ddd2ZwcwLRhqKUa8AzHG3s+ZZYhabMMYbnVP8G+430i8XbqUJ1EZ+2Rxj0fHVm4QCeC 4W3wv0sDMJuMECuPPZ0PlOFchbtr2g2JEZpxdVNkogmey8cg+CTPGvsoTzuAnAxfC8T8 cSkw== X-Gm-Message-State: AOAM532zt3EvZYwcFv1XEjp5O0RAtFhBUpt8I+yskev78SvUMJsRuYzy rnB05tNTaNVtAuqViz3KNjBN46Q4nbbPPEIr7OYAlVf8iPs= X-Google-Smtp-Source: ABdhPJzbFoLFyatgFkbYhq2Vjjcy38uIqzb7M9zdLCfEsJpPIQzfLnwv/kw8YUkWa5KTJDOuaxWWt/3j2teu4HIzpcQ= X-Received: by 2002:aca:f456:: with SMTP id s83mr2940283oih.58.1610640153530; Thu, 14 Jan 2021 08:02:33 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: "H.J. Lu" Date: Thu, 14 Jan 2021 08:01:57 -0800 Message-ID: Subject: Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] To: Szabolcs Nagy Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3036.4 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.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 14 Jan 2021 16:02:35 -0000 On Thu, Jan 14, 2021 at 7:52 AM H.J. Lu wrote: > > On Thu, Jan 14, 2021 at 7:49 AM H.J. Lu wrote: > > > > On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu wrote: > > > > > > On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha > > > wrote: > > > > > > > > IFUNC resolvers may depend on tunables and cpu feature setup so > > > > move static pie self relocation after those. > > > > > > > > It is hard to guarantee that the ealy startup code does not rely > > > > on relocations so this is a bit fragile. It would be more robust > > > > to handle RELATIVE relocs early and only IRELATIVE relocs later, > > > > but the current relocation processing code cannot do that. > > > > > > > > The early startup code before relocation processing includes > > > > > > > > _dl_aux_init (auxvec); > > > > __libc_init_secure (); > > > > __tunables_init (__environ); > > > > ARCH_INIT_CPU_FEATURES (); > > > > > > > > These are simple enough that RELATIVE relocs can be avoided. > > > > > > > > __ehdr_start may require RELATIVE relocation so it was moved > > > > later, fortunately ehdr and phdr are not used in the early code. > > > > --- > > > > csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- > > > > 1 file changed, 25 insertions(+), 19 deletions(-) > > > > > > > > diff --git a/csu/libc-start.c b/csu/libc-start.c > > > > index db859c3bed..fb64cdb2c9 100644 > > > > --- a/csu/libc-start.c > > > > +++ b/csu/libc-start.c > > > > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > int result; > > > > > > > > #ifndef SHARED > > > > - _dl_relocate_static_pie (); > > > > - > > > > char **ev = &argv[argc + 1]; > > > > > > > > __environ = ev; > > > > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > } > > > > # endif > > > > _dl_aux_init (auxvec); > > > > - if (GL(dl_phdr) == NULL) > > > > # endif > > > > - { > > > > - /* Starting from binutils-2.23, the linker will define the > > > > - magic symbol __ehdr_start to point to our own ELF header > > > > - if it is visible in a segment that also includes the phdrs. > > > > - So we can set up _dl_phdr and _dl_phnum even without any > > > > - information from auxv. */ > > > > - > > > > - extern const ElfW(Ehdr) __ehdr_start > > > > - __attribute__ ((weak, visibility ("hidden"))); > > > > - if (&__ehdr_start != NULL) > > > > - { > > > > - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > > > - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > > > - GL(dl_phnum) = __ehdr_start.e_phnum; > > > > - } > > > > - } > > > > > > > > /* Initialize very early so that tunables can use it. */ > > > > __libc_init_secure (); > > > > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > > > > > ARCH_INIT_CPU_FEATURES (); > > > > > > > > + /* Do static pie self relocation after tunables and cpu features > > > > + are setup for ifunc resolvers. Before this point relocations > > > > + must be avoided. */ > > > > + _dl_relocate_static_pie (); > > > > + > > > > /* Perform IREL{,A} relocations. */ > > > > ARCH_SETUP_IREL (); > > > > > > > > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > hwcap and platform fields available in the TCB. */ > > > > ARCH_APPLY_IREL (); > > > > > > > > +# ifdef HAVE_AUX_VECTOR > > > > + if (GL(dl_phdr) == NULL) > > > > +# endif > > > > + { > > > > + /* Starting from binutils-2.23, the linker will define the > > > > + magic symbol __ehdr_start to point to our own ELF header > > > > + if it is visible in a segment that also includes the phdrs. > > > > + So we can set up _dl_phdr and _dl_phnum even without any > > > > + information from auxv. */ > > > > + > > > > + extern const ElfW(Ehdr) __ehdr_start > > > > + __attribute__ ((weak, visibility ("hidden"))); > > > > + if (&__ehdr_start != NULL) > > > > + { > > > > + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > > > + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > > > + GL(dl_phnum) = __ehdr_start.e_phnum; > > > > + } > > > > + } > > > > + > > > > /* Set up the stack checker's canary. */ > > > > uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > > > > # ifdef THREAD_SET_STACK_GUARD > > > > -- > > > > 2.17.1 > > > > > > > > > > LGTM. > > > > > > Thanks. > > > > > > > Unfortunately, this failed on i686: > > > > (gdb) r > > Starting program: > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/elf/sln > > > > Program received signal SIGSEGV, Segmentation fault. > > 0xefec0550 in ?? () > > > > Breakpoint 1, __libc_start_main (main=0xf7f64760
, argc=1, > > argv=0xffffc704, init=0xf7f670e0 <__libc_csu_init>, > > fini=0xf7f67190 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0xffffc6fc) > > at ../csu/libc-start.c:145 > > 145 char **ev = &argv[argc + 1]; > > (gdb) next > > 147 __environ = ev; > > (gdb) > > 151 __libc_stack_end = stack_end; > > (gdb) > > 160 while (*evp++ != NULL) > > (gdb) > > 165 _dl_aux_init (auxvec); > > (gdb) > > 169 __libc_init_secure (); > > (gdb) > > 171 __tunables_init (__environ); > > (gdb) > > 173 ARCH_INIT_CPU_FEATURES (); > > (gdb) > > 178 _dl_relocate_static_pie (); > > (gdb) > > 181 ARCH_SETUP_IREL (); > > (gdb) > > 184 ARCH_SETUP_TLS (); > > (gdb) > > 203 if (&__ehdr_start != NULL) > > (gdb) > > 212 uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > > (gdb) > > 223 DL_SYSDEP_OSCHECK (__libc_fatal); > > (gdb) > > > > Program received signal SIGSEGV, Segmentation fault. > > 0xefec0550 in ?? () > > (gdb) > > > > -- > > H.J. > > (gdb) si > uname () at ../sysdeps/unix/syscall-template.S:120 > 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > (gdb) si > 0xf7fba3a2 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > (gdb) si > 0xf7fba3a6 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > (gdb) si > 0xf7fba3ab 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > (gdb) si > 0xefec0550 in ?? () > (gdb) disass uname > Dump of assembler code for function uname: > 0xf7fba3a0 <+0>: mov %ebx,%edx > 0xf7fba3a2 <+2>: mov 0x4(%esp),%ebx > 0xf7fba3a6 <+6>: mov $0x7a,%eax > 0xf7fba3ab <+11>: call *%gs:0x10 <<<<<<<<<<<< This may not be setup yet. > 0xf7fba3b2 <+18>: mov %edx,%ebx > 0xf7fba3b4 <+20>: cmp $0xfffff001,%eax > 0xf7fba3b9 <+25>: jae 0xf7f9efd0 <__syscall_error> > 0xf7fba3bf <+31>: ret > End of assembler dump. > (gdb) > > > -- > H.J. GL(dl_sysinfo) points to the wrong address. This may affect all variables accessed in _dl_aux_init. -- H.J.