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ł