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 c198b590 for ; Fri, 31 Jan 2020 17:51:30 +0000 (UTC) Received: (qmail 11837 invoked by uid 550); 31 Jan 2020 17:51:28 -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 11816 invoked from network); 31 Jan 2020 17:51:28 -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=0BN8BbMj9OTegNrafNQ3KAejprNv2TaVtoAkJclCd5w=; b=kn6J/pvCQHexPEe1rG5oGxaf+YWEg9RoVZG/9J/TwM2GPp4LsnndgRTVnBQpjvrpsK FI3phHG+oeiu71dSKv/dydEs0xH3Cs82XAjBZTIsm7Em+IdN51lRP74vMtuW6b/M2btZ FzeVzYxPh9VN6XBxpTNPj8EM/v/HZQTuG6hx6RirbzlAmHCiAO9PkcqOQidtJUcDojlz vN8LkcuhNeq5N9Mtd9gUdW7FmOWfnBc+YJVBEfSjHTmzBG4cwnLoBBVlLKFBTofH+j1P 1K1jVWR8WCCzmopflNCphutTveL8+ud2fzrp1snW28sac7BRSUeff8c9k9pcnYHh2Fx7 /rNA== 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=0BN8BbMj9OTegNrafNQ3KAejprNv2TaVtoAkJclCd5w=; b=AlynayLsEO6QZwvITeXinUwtIEyGz27OFrRpIvYxRpkGWI89XDyjWq8i53k87jwlAX tS4uQDthgABHI+V7tqkZa+Ghij+1+LUBA5xCpS/YwwiCp1Kw7BSZob+zXAIWoN9Q9j8D sAlEwMcpJusEJKTJjqO627YIaoo6BPUHSBnL6hZzSCfYLzbBdiVBxxPPUTVYAleKux7X 7qc4BEa9Ltfsc/k0kxjSfuVy+QLR1bP/hsGwlLiDPvwL2ey5FOsIO6SV1PNqM/UMOIDd YffNl041VGONpds9FBBxT8FKa08ksGCy+dT4FgdiiZdnpVeG5jXFx9nRZnKm/MJ5qW6x QDgQ== X-Gm-Message-State: APjAAAUpIitZweOysuGZv7K97hAdahDU2fvRB2J1nYmO52AntEax6hUS eozQlyP4KAwA0yQlkPkK7HaaD84M7msbOWx44CCn/7Qd8Bo= X-Google-Smtp-Source: APXvYqyCZuF4tHUNFtSQUbz//2eCAt/wjXrnu6T6U3hCpZDtisOfk6vpF+hYWXdc8C4ojps/NTi4yWH4ik096S9mO8o= X-Received: by 2002:a2e:85ce:: with SMTP id h14mr6669121ljj.41.1580493076552; Fri, 31 Jan 2020 09:51:16 -0800 (PST) MIME-Version: 1.0 References: <20200129191946.GI2020@voyager> <20200130170249.GK2020@voyager> <20200131164009.GI1663@brightrain.aerifal.cx> In-Reply-To: <20200131164009.GI1663@brightrain.aerifal.cx> From: =?UTF-8?B?0JDQvdC00YDQtdC5INCQ0LvQsNC00YzQtdCy?= Date: Fri, 31 Jan 2020 20:51:05 +0300 Message-ID: To: musl@lists.openwall.com Content-Type: multipart/alternative; boundary="00000000000065ddd0059d73387e" Subject: Re: [musl] Static linking is broken after creation of DT_TEXTREL segment --00000000000065ddd0059d73387e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > Moreover, adding such a thing is not desirable because it adds a linear-search of an array of load segments to each relocation I think it is possible to make this search O(log n). for (j=3D0; v-p->loadmap->segs[j].p_vaddr >=3D p->loadmap->segs[j].p_memsz; j++); This code takes first segment that can handle address. It looks possible to create modified list of virtual segments, that won't overlap and make a binary search. > This is not a reasonable tradeoff and it's why I proposed the "hull" approach It may not be safe from maintainability perspective. Library will be changed, everybody will forget what part of code was protected by external validation and it will provide unexpected behaviour. Please consider double validation approach: external validation in production and debug only mode with loadmap like validation. =D0=BF=D1=82, 31 =D1=8F=D0=BD=D0=B2. 2020 =D0=B3. =D0=B2 19:40, Rich Felker= : > On Fri, Jan 31, 2020 at 06:16:19PM +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: > > 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 the= y > > 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; > > This is only for FDPIC, which is not used on any platforms you care > about unless you already know what it is. :-) > > > 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; > > }; > > No you can't, because this structure is ABI between the loader (kernel > or otherwise) and the program. It's not an internal structure of the > dynamic linker. > > Moreover, adding such a thing is not desirable because it adds a > linear-search of an array of load segments to each relocation, > increasing load time of every correct program for the sake of > diagnosing incorrect ones in a friendly manner. This is not a > reasonable tradeoff and it's why I proposed the "hull" approach > (that's O(1) and safe against bogus relocations clobbering memory > outside the range of the library being relocated, but could still > fault with a mix of LOAD maps). The FDPIC approach is only done > because it's essential for being able to share program text on a > system without MMU, not because it's "nice". > > Rich > --00000000000065ddd0059d73387e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
> Moreover, adding such a thing is not desirable becaus= e it adds a linear-search of an array of load segments to each relocation
I think it is possible to make this search O(log n).

for (j=3D0; v-p->loadmap->segs[j].p_vaddr >= =3D p->loadmap->segs[j].p_memsz; j++);

T= his code takes first segment that can handle address. It looks possible to = create modified list of virtual segments, that won't overlap and make a= binary search.

> This is not a reasonable trad= eoff and it's why I proposed the "hull" approach

It may not be safe from=C2=A0maintainability perspective. = Library will be changed, everybody will forget what part of code was protec= ted by external validation and it will provide unexpected behaviour. Please= consider double validation approach: external validation in production and= debug only mode with loadmap like validation.

=D0=BF=D1=82, 31 =D1=8F= =D0=BD=D0=B2. 2020 =D0=B3. =D0=B2 19:40, Rich Felker <dalias@libc.org>:
On Fri, Jan 31, 2020 at 06:16:19PM +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 w= rote:
> Hello Markus, I want to ask a question about this one:
>
> > The issue is more complicated, because the app can have an unboun= ded
> number of PT_LOAD segments with the PF_W flag absent. So checking the<= br> > relocations would require the dynlinker to first iterate over all PHDR= s to
> check for the unlikely case that textrels are present. Only because th= ey
> 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;

This is only for FDPIC, which is not used on any platforms you care
about unless you already know what it is. :-)

> 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 {
>=C2=A0 =C2=A0uintptr_t addr, p_vaddr, p_memsz;
>=C2=A0 =C2=A0bool readonly;
> };

No you can't, because this structure is ABI between the loader (kernel<= br> or otherwise) and the program. It's not an internal structure of the dynamic linker.

Moreover, adding such a thing is not desirable because it adds a
linear-search of an array of load segments to each relocation,
increasing load time of every correct program for the sake of
diagnosing incorrect ones in a friendly manner. This is not a
reasonable tradeoff and it's why I proposed the "hull" approa= ch
(that's O(1) and safe against bogus relocations clobbering memory
outside the range of the library being relocated, but could still
fault with a mix of LOAD maps). The FDPIC approach is only done
because it's essential for being able to share program text on a
system without MMU, not because it's "nice".

Rich
--00000000000065ddd0059d73387e--