[U-Boot] env: fix potential stack overflow in environment functions

[Tom Rini]

On Fri, Apr 05, 2013 at 01:16:36PM -0500, Rob Herring wrote:
> On 04/05/2013 12:13 PM, Wolfgang Denk wrote:
> > Dear Rob,
> > 
> > In message <515EFE6F.1020609 at gmail.com> you wrote:
> >>
> >>> In addition to commit 60d7d5a "env: fix potential stack overflow in
> >>> environment functions" discussed here, I think we should also revert
> >>> commit fcfa696 "ARM: increase lmb stack space reservation to 4KB"
> >>> because it is conceptually broken and just papers over the real
> >>> problems.
> >>
> >> Doing so will randomly break any system with a large command or print
> >> buffer. For extra fun, it is dependent on the initrd or dtb image size
> >> in terms of remainder of 4KB multiple.
> > 
> > Well, yes, but that's because the LMB code makes unjustified
> > assumptions about the memory usage, so it needs to be fixed there.
> > 
> >> It is exactly the same code as PPC. It you look at the git history, PPC
> >> made exactly the same change (1 to 4KB increase) around the same time
> >> all the FDT boot code got copied from PPC to ARM. So ARM missed this change.
> > 
> > Thanks for pointing out.  This adds commit 3882d7a "ppc: unused memory
> > region too close to current stack pointer" to the list of patches that
> > should bne reverted.
> > 
> >> If the stack is all of RAM, then what address should the initrd and dtb
> >> be copied to?
> > 
> > Why do they have to be copied at all?  Why cannot they remain where
> > they have been loaded in the firtst place?  The memcpy just costs time,
> > which is a precious resource.  Leave it to the user to find a
> > reasonable location in RAM where he loads the data, and don't mess
> > with it.
> 
> I've got no freaking idea! I do turn that crap off in my environment
> with initrd_high=0xffffffff. But the default operation is to copy it.

There's also fdt_high=0xffffffff which a number of platforms do to keep
from FDT being thrown into highmem and unavailable to Linux.

So, I shall revert the first commit in question today.  For after
v2013.04 we should:
- Revert the other two commits Wolfgang found
- See if there looks to be a real good reason for defaulting to
  relocating initrd/fdt, specifically up into the top of memory where we
  also know that U-Boot has / is alive and kicking.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130405/881773c3/attachment.pgp>