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=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 9623 invoked from network); 25 Sep 2020 09:37:49 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 25 Sep 2020 09:37:49 -0000 Received: (qmail 25700 invoked by uid 550); 25 Sep 2020 09:37:45 -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 25659 invoked from network); 25 Sep 2020 09:37:45 -0000 Date: Fri, 25 Sep 2020 11:37:33 +0200 From: Szabolcs Nagy To: Dominic Chen Cc: musl@lists.openwall.com Message-ID: <20200925093733.GJ2947641@port70.net> Mail-Followup-To: Dominic Chen , musl@lists.openwall.com References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [musl] SIGSEGV with TEXTREL * Dominic Chen [2020-09-24 23:50:19 -0400]: > Please CC me on replies. > > I recently discovered that musl doesn't support DT/DF_TEXTREL in the > main executable, which can result in the dynamic loader crashing with > SIGSEGV and SEGV_ACCERR while processing relocations. I spent a few days > trying to fix this in the toolchain, but because it is a prototype based > on Clang/LLVM 4.0.0 that adds runtime instrumentation built using the > x64 large code model, so it's not easy to fix. Also, glibc does support > this behavior. there are no existing libcs that fully support textrels (since for that not just dynamic relocs but static relocs need to be supported too). glibc only supports a small set of textrels and of course it has to mark the code executable+writable which means (1) the code cannot be shared across processes, it will actually use physical memory where the modified code is stored per process which is not ideal when you work with large code model, (2) all security policies have to be turned off that prevent exec+write mappings for this to work at all which is not acceptable in many environments. for these reasons it is considered to be a bug to create binaries with textrels. i think large code model should not need textrel on x86_64: there should be a way to create >4G pc relative offset in code that does not need any relocs. (or do you have some example where that fails?) > > I ended up implementing support for this in musl itself (patch > attached), but given the discussion in the previous thread, "Static > linking is broken after creation of DT_TEXTREL," it seems like this > isn't acceptable due to overhead? I don't quite understand the concern, > because the loader needs to iterate again over the program headers only dynamic linking overhead is not the issue. > if the program contains TEXTRELs, which is strictly an improvement, even > if the iteration itself is suboptimal. Alternatively, I'd suggest that > musl at least warns about unsupported TEXTRELs if present, because > asking application developers to debug a crashing ELF loader is quite a > high bar. dynamic linker failure diagnostic is something musl could improve i think. > > Thanks, > > Dominic > > diff --git a/ldso/dynlink.c b/ldso/dynlink.c > index d7726118..c7449df2 100644 > --- a/ldso/dynlink.c > +++ b/ldso/dynlink.c > @@ -1326,10 +1326,32 @@ static void do_mips_relocs(struct dso *p, size_t *got) > > static void reloc_all(struct dso *p) > { > + unsigned char textrel = 0; > size_t dyn[DYN_CNT]; > for (; p; p=p->next) { > if (p->relocated) continue; > decode_vec(p->dynv, dyn, DYN_CNT); > + > + if ((dyn[0] & 1< + size_t cnt = p->phnum; > + Phdr *ph = p->phdr; > + for (; cnt--; ph = (void *)((char *)ph + p->phentsize)) { > + if (ph->p_type == PT_LOAD && !(ph->p_flags & PF_W)) { > + unsigned prot = (((ph->p_flags&PF_R) ? PROT_READ : 0) | > + ((ph->p_flags&PF_X) ? PROT_EXEC : 0)); > + size_t start = ph->p_vaddr & -PAGE_SIZE, > + end = (ph->p_vaddr + ph->p_memsz + PAGE_SIZE-1) & -PAGE_SIZE; > + if (mprotect(laddr(p, start), end - start, prot|PROT_WRITE) > + && errno != ENOSYS) { > + error("Error relocating %s: TEXTREL unprotect failed: %m", > + p->name); > + if (runtime) longjmp(*rtld_fail, 1); > + } > + textrel = 1; > + } > + } > + } > + > if (NEED_MIPS_GOT_RELOCS) > do_mips_relocs(p, laddr(p, dyn[DT_PLTGOT])); > do_relocs(p, laddr(p, dyn[DT_JMPREL]), dyn[DT_PLTRELSZ], > @@ -1345,6 +1367,25 @@ static void reloc_all(struct dso *p) > if (runtime) longjmp(*rtld_fail, 1); > } > > + if (textrel) { > + size_t cnt = p->phnum; > + Phdr *ph = p->phdr; > + for (; cnt--; ph = (void *)((char *)ph + p->phentsize)) { > + if (ph->p_type == PT_LOAD && !(ph->p_flags & PF_W)) { > + unsigned prot = (((ph->p_flags&PF_R) ? PROT_READ : 0) | > + ((ph->p_flags&PF_X) ? PROT_EXEC : 0)); > + size_t start = ph->p_vaddr & -PAGE_SIZE, > + end = (ph->p_vaddr + ph->p_memsz + PAGE_SIZE-1) & -PAGE_SIZE; > + if (mprotect(laddr(p, start), end - start, prot) > + && errno != ENOSYS) { > + error("Error relocating %s: TEXTREL protect failed: %m", > + p->name); > + if (runtime) longjmp(*rtld_fail, 1); > + } > + } > + } > + } > + > p->relocated = 1; > } > }