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

[Rob Herring <robherring2@gmail.com>]

On 03/22/2013 05:04 PM, Wolfgang Denk wrote:
> Dear Rob Herring,
> 
> In message <1363987581-28050-1-git-send-email-robherring2@gmail.com> you wrote:
>>
>> Most of the various environment functions create CONFIG_ENV_SIZE buffers on
>> the stack. At least on ARM and PPC which have 4KB stacks, this can overflow
>> the stack if we have large environment sizes. So move all the buffers off
>> the stack to static buffers.
> 
> Could you please explain what exactly you mean with this "have 4KB
> stacks" statement?

Well, that's what ARM and PPC have for their stack size. This is not the
first stack overflow I've fixed. The last was caused by having a large
printf buffer size:

commit fcfa696b3a354ab1e16601683c61f671487aded7
Author: Rob Herring <rob.herring@calxeda.com>
Date:   Thu Jun 28 03:54:11 2012 +0000

    ARM: increase lmb stack space reservation to 4KB

    The bootm initrd image copy to ram can collide with the stack in cases
    where the print buffer size is large (i.e. 1K). The result is
intermittent
    initrd decompression errors depending on the initrd size MOD 4KB since
    the initrd start address is 4KB aligned.

    Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
    Signed-off-by: Rob Herring <rob.herring@calxeda.com>

> Also, why do you think there is more space for data then for stack?

Because in "normal" systems that is the case. It is reasonable to expect
that a 1MB allocation would be possible for static data or malloc, but
not for a stack. I could have just increased the stack size, but that is
a fragile solution.

> Has this patch been tested on any actual hardware?

Yes. This fixes a problem on actual h/w (highbank). The problem was
introduced with only an env change.

> I would expect problems, as this code is running before relocation,
> i. e. when the data segment is not writable?

env_relocate is called by board_init_r at least on ARM which is after
relocation, right?

Rob