Messages in this thread | | | Date | Wed, 9 Apr 2008 10:59:40 -0700 | From | "Yinghai Lu" <> | Subject | Re: [PATCH 1/2] boot: increase stack size for kernel boot loader decompressor |
| |
On Wed, Apr 9, 2008 at 8:08 AM, Alexander van Heukelum <heukelum@fastmail.fm> wrote: > > On Tue, 8 Apr 2008 10:54:15 -0700, "Yinghai Lu" <yhlu.kernel@gmail.com> > said: > > > On Tue, Apr 8, 2008 at 1:23 AM, Ingo Molnar <mingo@elte.hu> wrote: > > > > > > * Alexander van Heukelum <heukelum@mailshack.com> wrote: > > > > > > > I did see that the malloc space that the inflate code is using is > > > > taken from _after_ the end of the bss. I don't see how this is > > > > protected from being used/overwritten. Changing the stack size changes > > > > the memory layout a bit... maybe you were so unlucky to create a > > > > vmlinux image that was just barely smaller than some threshold and > > > > increasing the stack size made the decompression/relocation area be > > > > located somewhere else? > > > > > > > > Test patch follows. > > > > > > that's a really interesting theory. > > > > > > FWIIW, i've been booting allyesconfig bzImages for a long time (with > > > only minimal amount of drivers disabled - mostly old ISA ones that > > > assume the presence of the real hardware), and they boot and work fine > > > on both 32-bit and 64-bit typical whitebox PCs. That means huge bzImages > > > that decompresses into a ~41 MB kernel image. I'd expect that to be a > > > rather severe test of the decompressor. > > > > i don't that Alexander's patch is needed. > > Hello Yinghai Lu, > > Indeed, I now think it is not needed either. The decompression is > done in-place nowadays: the (compressed) image is moved to a high > memory address first, then the decompression is done starting at > the low end of the buffer. It is guaranteed that the output never > overwrites the input, and the decompression code, the stack, and > the heap are all at higher addresses than the input buffer. The > same goes for the pagetables needed for x86_64. > > > > also because Alex move heap before _end, > > we may need add some extra for buffer offset > > > > /* Replace the compressed data size with the uncompressed size */ > > subl input_len(%ebp), %ebx > > movl output_len(%ebp), %eax > > addl %eax, %ebx > > /* Add 8 bytes for every 32K input block */ > > shrl $12, %eax > > addl %eax, %ebx > > /* Add 32K + 18 bytes of extra slack and align on a 4K boundary > > */ > > addl $(32768 + 18 + 4095), %ebx > > andl $~4095, %ebx =============================> need add > > heap size too. > > .... > > No, that size is accounted for automatically: the code computes the > buffer size needed (including slack) minus the buffer size that is > already available (in the embedded gzip-file). The image is moved > by this amount (rounded up to a page). So that part is fine.
just wonder if Ingo have very big vmlinux, that +32K + 18 formula still works.
YH
| |