[haiku-commits] Re: haiku: hrev47798 - headers/private/system src/system/boot/loader
- From: Paweł Dziepak <pdziepak@xxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Tue, 2 Sep 2014 19:58:45 +0200
2014-09-02 19:09 GMT+02:00 pulkomandy <pulkomandy@xxxxxxxxxxxxx>:
> On Tue, Sep 02, 2014 at 06:51:45PM +0200, Paweł Dziepak wrote:
> > 2014-09-02 18:24 GMT+02:00 <pulkomandy@xxxxxxxxxxxxx>:
> > <snip>
> >
> > > diff --git a/src/system/boot/loader/elf.cpp
> > > b/src/system/boot/loader/elf.cpp
> > > index 7479463..ef06097 100644
> > > --- a/src/system/boot/loader/elf.cpp
> > > +++ b/src/system/boot/loader/elf.cpp
> > >
> > <snip>
> >
> > > @@ -275,7 +277,8 @@ ELFLoader<Class>::Load(int fd, preloaded_image*
> _image)
> > > // inbetween.
> > > totalSize = secondRegion->start + secondRegion->size -
> > > firstRegion->start;
> > > if (totalSize > image->text_region.size +
> image->data_region.size
> > > - + 16 * 1024) {
> > > + + 32 * 1024) {
> > > + dprintf("Too much space between segments %" B_PRIuSIZE
> > > "!\n", totalSize);
> > > status = B_BAD_DATA;
> > > goto error1;
> > > }
> > >
> >
> > To be honest, I don't think that there is any point in having this check.
> > IIRC ELF specification doesn't impose any limit on the amount of unused
> > space between "text region" and "data region", so it is wrong to do that
> in
> > our code.
>
> It seems that the quite limited elf loader used at boot time expects to
> be able to put each ELF file inside a single linear memory region, and
> that needs reasonably close addresses for all sections in the file
> (otherwise it would run out of address space quickly).
>
Right, the temporary MMU management has its limitations which may make the
bootloader unable to load a perfectly valid ELF file. That still doesn't
explain the existence of this check. If the loadable segments are not
"reasonably close" then AllocateRegion would fail - that's ok, a sufficient
memory region cannot be allocated to successfully load an ELF file in such
restricted conditions, so we fail. Why add another check which is actually
only a guess (as your patch shows, it was, and probably still is, stricter
than necessary) what kind of ELF files are acceptable?
Paweł
Other related posts:
