From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 28910 invoked from network); 2 Jul 2022 20:16:40 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 2 Jul 2022 20:16:40 -0000 Received: (qmail 17986 invoked by uid 550); 2 Jul 2022 20:16:36 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 17945 invoked from network); 2 Jul 2022 20:16:35 -0000 Date: Sat, 2 Jul 2022 16:16:23 -0400 From: Rich Felker To: "Alex Xu (Hello71)" Cc: musl@lists.openwall.com Message-ID: <20220702201623.GW7074@brightrain.aerifal.cx> References: <20220330021148.520601-1-alex_y_xu.ref@yahoo.ca> <20220330021148.520601-1-alex_y_xu@yahoo.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220330021148.520601-1-alex_y_xu@yahoo.ca> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH] before __dls2b, call mprotect with __syscall On Thu, Jun 30, 2022 at 01:52:43PM -0400, Alex Xu (Hello71) wrote: > if LTO is enabled, gcc hoists the call to ___errno_location outside the > loop even though the access to errno is gated behind head != &ldso > because ___errno_location is marked __attribute__((const)). this causes > the program to crash because TLS is not yet initialized when called from > __dls2. this is also possible if LTO is not enabled; even though gcc 11 > doesn't do it, it is still wrong to use errno here. > > since the start and end are already aligned, we can simply call > __syscall instead of using global errno. > > Fixes: e13a2b8953ef ("implement PT_GNU_RELRO support") > --- > > ldso/dynlink.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/ldso/dynlink.c b/ldso/dynlink.c > index 5b9c8be4..b5a8f1f2 100644 > --- a/ldso/dynlink.c > +++ b/ldso/dynlink.c > @@ -1356,12 +1356,14 @@ static void reloc_all(struct dso *p) > do_relocs(p, laddr(p, dyn[DT_REL]), dyn[DT_RELSZ], 2); > do_relocs(p, laddr(p, dyn[DT_RELA]), dyn[DT_RELASZ], 3); > > - if (head != &ldso && p->relro_start != p->relro_end && > - mprotect(laddr(p, p->relro_start), p->relro_end-p->relro_start, PROT_READ) > - && errno != ENOSYS) { > - error("Error relocating %s: RELRO protection failed: %m", > - p->name); > - if (runtime) longjmp(*rtld_fail, 1); > + if (head != &ldso && p->relro_start != p->relro_end) { > + long ret = __syscall(SYS_mprotect, laddr(p, p->relro_start), > + p->relro_end-p->relro_start, PROT_READ); > + if (ret != 0 && ret != -ENOSYS) { > + error("Error relocating %s: RELRO protection failed: %m", > + p->name); > + if (runtime) longjmp(*rtld_fail, 1); > + } > } > > p->relocated = 1; > -- > 2.35.1 I think it's ok to go ahead and make this change since it's an improvement, but it looks like it's not actually a complete fix, because the call to error() is still there, which *itself* has a path to evaluation of errno via printf. A complete fix probably needs to make reloc_all take a function pointer to the error function as an argument, with __dls2 passing in a dummy function that just crashes. Rich