[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 31 Jan 2020 20:51:05 +0300
From: Андрей Аладьев <aladjev.andrew@...il.com>
To: musl@...ts.openwall.com
Subject: Re: Static linking is broken after creation of DT_TEXTREL segment
> 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=0; v-p->loadmap->segs[j].p_vaddr >= 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.
пт, 31 янв. 2020 г. в 19:40, Rich Felker <dalias@...c.org>:
> On Fri, Jan 31, 2020 at 06:16:19PM +0300, Андрей Аладьев 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 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 = ph->p_vaddr;
> > dso->loadmap->segs[i].p_memsz = 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 = 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
>
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
