From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.2 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by inbox.vuxu.org (OpenSMTPD) with SMTP id 9218bd31 for ; Fri, 31 Jan 2020 15:16:44 +0000 (UTC) Received: (qmail 14259 invoked by uid 550); 31 Jan 2020 15:16:42 -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 14241 invoked from network); 31 Jan 2020 15:16:42 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=tPjWMw2A0I523gy1CJ7mzBKgZMKspu+5x2DOWvh/GrE=; b=AYjJv4qhVT20OX4AkedliAGZn9G3+nsU1mKENKMgYDJ4xtNaS71xHPFvWi4qm3bUHK E+BeNvcS1k/9KvR3FCXW7UE6JzHVFgU1Ns6FE0CSDweC/C/PjefA1J+/vQ1kEfRumsBJ zXOOYDuj18BtwdKg5JnkNEp3ap4Pnkr6zDPfN5/pJ07dbOC3Me5VEXh8Msco5+/twsgk MRC+o6UlJ36BUa1czhQMQcqV9AVHLBpsS34QLYQVRum2Mnuq/U+OwZsL6yrqB92M5hvN jjJP9TvnuEsV+GJZuy7uM7yM8AQsKEXQ89HeOPA1PH+2zQe4L9BBpQnwCGn/DwBqMvJP je2g== 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; bh=tPjWMw2A0I523gy1CJ7mzBKgZMKspu+5x2DOWvh/GrE=; b=bq+XAjsPjf8cteeFz4CM/GsZsDw0aBIFdJ8DkUvjX6hQy4YShACY2Bo3b2fTM4RNoS GOELm9G2T1pLxyTtJGIr3BQPboVDEMFAlBt0mw7xAIfX1aDlZIPOZgq1h1040U+6JUlz wmLDlJ8sKZG/sbTO8dys6GeLm031rMwzjeI5kuCexeT2an4TrWLryfMqCpfMtQ1BFdeq C+0FcTVo392x9GGOZYu27pobUN6CYRUWeNGUe+EdkA7JKTAzQQwK+wNxrINGXILqWZo8 q20wgkSTAeMmb2CjN+kP+AcIKldw6x64HWeWF1C3c/qFX6qk1KV50HXhb6ZlO7PVo1px 4EsA== X-Gm-Message-State: APjAAAVMhKLwPNk7YAatOWdFYg4vNCGJSRfeROMXy32clPgvS28pbVAH TyScOeqHnFemw4JhMU5d71H91uTBKSndTzDGoqHKp7T4KWM= X-Google-Smtp-Source: APXvYqzyDOKRNwZin6PHS0YVhl5noL6SHuygf7SG6qzuXan/4VY4x1AK38BSypLu3TgR05baNqoNzImrpwI9B8Vtn0I= X-Received: by 2002:a19:5201:: with SMTP id m1mr5719886lfb.114.1580483790446; Fri, 31 Jan 2020 07:16:30 -0800 (PST) MIME-Version: 1.0 References: <20200129191946.GI2020@voyager> <20200130170249.GK2020@voyager> In-Reply-To: <20200130170249.GK2020@voyager> From: =?UTF-8?B?0JDQvdC00YDQtdC5INCQ0LvQsNC00YzQtdCy?= Date: Fri, 31 Jan 2020 18:16:19 +0300 Message-ID: To: musl@lists.openwall.com Content-Type: multipart/alternative; boundary="000000000000e72406059d710eba" Subject: Re: [musl] Static linking is broken after creation of DT_TEXTREL segment --000000000000e72406059d710eba Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hello Markus, I want to ask a question about this one: > The issue is more complicated, because the app can have an unbounded number of PT_LOAD segments with the PF_W flag absent. So checking the relocations would require the dynlinker to first iterate over all PHDRs to check for the unlikely case that textrels are present. Only because they might be. I made a light code overview and found that there is already a mapping for segments: "loadmap": dso->loadmap->segs[i].p_vaddr =3D ph->p_vaddr; dso->loadmap->segs[i].p_memsz =3D ph->p_memsz; We can add here the following line: dso->loadmap->segs[i].readonly =3D ph->p_flags & PF_W; Than add "readonly" into "fdpic_loadseg": struct fdpic_loadseg { uintptr_t addr, p_vaddr, p_memsz; bool readonly; }; Than we can refactor "do_relocs" function, it will use new function like "reloc_addr =3D request_writable_address(dso, rel[0])" instead of "laddr". = It will return "NULL" if address is out of segments or readonly and throw beautiful error. But I found that "loadmap" is used only when "DL_FDPIC" is "1". But this is true only for "sh" arch. Why? This loadmap is very good abstraction the can live in separate file and be used everywhere. =D1=87=D1=82, 30 =D1=8F=D0=BD=D0=B2. 2020 =D0=B3. =D0=B2 20:03, Markus Wich= mann : > On Wed, Jan 29, 2020 at 11:08:46PM +0300, =D0=90=D0=BD=D0=B4=D1=80=D0=B5= =D0=B9 =D0=90=D0=BB=D0=B0=D0=B4=D1=8C=D0=B5=D0=B2 wrote: > > > Ooh boy, why would you do this? When there's a perfectly good -lgmp > just > > waiting for you. > > > > Usage of "/usr/lib/libgmp.a" directly is not forbidden by any toolchain > or > > build system. Moreover it is recommended by cmake. You can google for > > "target_link_libraries( site:github.com" and found millions of software > > that uses something like: > > > > find_library(EXTERNAL_LIB) > > target_link_libraries(something ${EXTERNAL_LIB_FOUND}) > > > > Doesn't that search for the library in the correct mode? I thought cmake > was that smart... > > > > The warning is justified, you usually do not want to do this. With a > > TEXTREL, the code has to be mapped as writable, so now programming erro= rs > > and exploits can change the executable code. > > > > This is warning from ld, not from musl. Segfault is not an acceptable > > message from libc. Libc should not try to write into readonly pointer. > > > > > Well, the remedy is obvious: Get rid of the TEXTREL. > > > > Yes, I've found a workaround: "USE=3D'-asm' emerge -v1 gmp", assembly i= s > > broken, will report it to gmp upstream. But this is not a fix for the > issue. > > > > There are several ways to achieve this. Getting rid of the assembly is > one such way. In another answer you also advocated for --with-pic. My > solution would probably have been to patch the code to never emit text > relocations in the first place, which is achieved the same way in the > end. > > The issue of musl not supporting textrels in the application itself > remains, though. > > > > Iterate over the apps PHDRs and remove write protection from all RO > > segments? > > > > So libc knows that file is mapped as readonly and should not try to wri= te > > into readonly pointers. > > Almost all relocations point to writable memory. For obvious reasons. So > musl doesn't check this. > > The issue is more complicated, because the app can have an unbounded > number of PT_LOAD segments with the PF_W flag absent. So checking the > relocations would require the dynlinker to first iterate over all PHDRs > to check for the unlikely case that textrels are present. Only because > they might be. > > > Libc can do the following: > > > > 1. Ignore impossible relocations. > > 2. Add a warning to stderr and still ignore impossible relocations. > > 3. do abort, user will receive SIGABRT and understand that he uses libc > in > > a wrong way. > > > > Segfault is not an acceptable answer. > > > > Have I got news for you. Unlike glibc, musl does not indicate > irrecoverable state with a litany into stderr, but usually by calling > a_crash(), which will terminate the process by executing an illegal > instruction. This typically results in SIGILL being delivered, but on > some archs it is still a segfault. > > Also, there is at least one place in the dynlinker where, as I recall, > mmap() is being called directly, but rather than check for errors in the > return value, the value is just used, because all error returns cause > segfaults. > > And then there was the case of PowerPC's original ABI, now called the > BSS-PLT ABI, which expects the dynlinker to fill out the PLT at runtime, > which musl doesn't do. Trying to run a BSS-PLT binary with musl will > therefore also very quickly segfault. musl doesn't fill out the PLT, > because the ABI is old, a replacement has been in place since at least > 2003, and PowerPC would be the only arch to need something like this. > > Anyway, option 1 would leave the relocations unprocessed, typically > leading to invalid code references down the line, and therefore another > segfault. Option 2 is the same but wordier. Option 3 has a chance to be > subverted (user could block or ignore SIGABRT before executing the main > binary. With SIGSEGV or SIGILL, the user can block or ignore those, but > then the kernel will just kill the process outright if those conditions > arrise). > > Options 1 and 2 also have the undesirable effect of possibly only > crashing sometimes, not all the time. See the recent cuserid() thread > about why that is a problem. > > And the issue you have with musl, not gmp, still remains: TEXTREL in the > application remains unsupported. > > Ciao, > Markus > --000000000000e72406059d710eba Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hello Markus, I want to ask a question about this one:
> The issue is more complicated, because the app can hav= e an unbounded number of PT_LOAD segments with the PF_W flag absent. So che= cking the relocations would require the dynlinker to first iterate over all= PHDRs to check for the unlikely case that textrels are present. Only becau= se they might be.

I made a light code overview and= found that there is already a mapping for segments: "loadmap":

dso->loadmap->segs[i].p_vaddr =3D ph->p_va= ddr;
dso->loadmap->segs[i].p_memsz =3D ph->p_memsz;

We can add here the following line:
dso->loa= dmap->segs[i].readonly =3D ph->p_flags & PF_W;

=
Than add "readonly" into "fdpic_loadseg":
struct fdpic_loadseg {
=C2=A0 uintptr_t addr, p_vaddr, p_memsz;<= /div>
=C2=A0 bool=C2=A0readonly;
};

<= div>Than we can refactor "do_relocs" function, it will use new fu= nction like "reloc_addr =3D request_writable_address(dso, rel[0])"= ; instead of "laddr". It will return "NULL" if address= =C2=A0is out of segments or readonly and throw beautiful error.
<= br>
But I found that "loadmap" is used only when "= DL_FDPIC" is "1". But this is true only for "sh" a= rch. Why? This loadmap is very good abstraction the can live in separate fi= le and be used everywhere.

=D1=87=D1=82, 30 =D1=8F=D0=BD=D0=B2. 2020 = =D0=B3. =D0=B2 20:03, Markus Wichmann <nullplan@gmx.net>:
On Wed, Jan 29, 2020 at 11:08:46PM +0300, =D0=90=D0=BD=D0=B4= =D1=80=D0=B5=D0=B9 =D0=90=D0=BB=D0=B0=D0=B4=D1=8C=D0=B5=D0=B2 wrote:
> > Ooh boy, why would you do this? When there's a perfectly good= -lgmp just
> waiting for you.
>
> Usage of "/usr/lib/libgmp.a" directly is not forbidden by an= y toolchain or
> build system. Moreover it is recommended by cmake. You can google for<= br> > "target_link_libraries( site:github.com" and found millions of s= oftware
> that uses something like:
>
> find_library(EXTERNAL_LIB)
> target_link_libraries(something ${EXTERNAL_LIB_FOUND})
>

Doesn't that search for the library in the correct mode? I thought cmak= e
was that smart...

> > The warning is justified, you usually do not want to do this. Wit= h a
> TEXTREL, the code has to be mapped as writable, so now programming err= ors
> and exploits can change the executable code.
>
> This is warning from ld, not from musl. Segfault is not an acceptable<= br> > message from libc. Libc should not try to write into readonly pointer.=
>
> > Well, the remedy is obvious: Get rid of the TEXTREL.
>
> Yes, I've found a workaround: "USE=3D'-asm' emerge -v= 1 gmp", assembly is
> broken, will report it to gmp upstream. But this is not a fix for the = issue.
>

There are several ways to achieve this. Getting rid of the assembly is
one such way. In another answer you also advocated for --with-pic. My
solution would probably have been to patch the code to never emit text
relocations in the first place, which is achieved the same way in the
end.

The issue of musl not supporting textrels in the application itself
remains, though.

> > Iterate over the apps PHDRs and remove write protection from all = RO
> segments?
>
> So libc knows that file is mapped as readonly and should not try to wr= ite
> into readonly pointers.

Almost all relocations point to writable memory. For obvious reasons. So musl doesn't check this.

The issue is more complicated, because the app can have an unbounded
number of PT_LOAD segments with the PF_W flag absent. So checking the
relocations would require the dynlinker to first iterate over all PHDRs
to check for the unlikely case that textrels are present. Only because
they might be.

> Libc can do the following:
>
> 1. Ignore impossible relocations.
> 2. Add a warning to stderr and still ignore impossible relocations. > 3. do abort, user will receive SIGABRT and understand that he uses lib= c in
> a wrong way.
>
> Segfault is not an acceptable answer.
>

Have I got news for you. Unlike glibc, musl does not indicate
irrecoverable state with a litany into stderr, but usually by calling
a_crash(), which will terminate the process by executing an illegal
instruction. This typically results in SIGILL being delivered, but on
some archs it is still a segfault.

Also, there is at least one place in the dynlinker where, as I recall,
mmap() is being called directly, but rather than check for errors in the return value, the value is just used, because all error returns cause
segfaults.

And then there was the case of PowerPC's original ABI, now called the BSS-PLT ABI, which expects the dynlinker to fill out the PLT at runtime, which musl doesn't do. Trying to run a BSS-PLT binary with musl will therefore also very quickly segfault. musl doesn't fill out the PLT, because the ABI is old, a replacement has been in place since at least
2003, and PowerPC would be the only arch to need something like this.

Anyway, option 1 would leave the relocations unprocessed, typically
leading to invalid code references down the line, and therefore another
segfault. Option 2 is the same but wordier. Option 3 has a chance to be
subverted (user could block or ignore SIGABRT before executing the main
binary. With SIGSEGV or SIGILL, the user can block or ignore those, but
then the kernel will just kill the process outright if those conditions
arrise).

Options 1 and 2 also have the undesirable effect of possibly only
crashing sometimes, not all the time. See the recent cuserid() thread
about why that is a problem.

And the issue you have with musl, not gmp, still remains: TEXTREL in the application remains unsupported.

Ciao,
Markus
--000000000000e72406059d710eba--