[U-Boot] memory corruption on nios2 due to overlap of gbl data and malloc

[Albert ARIBAUD]

Hi Graeme,

Le 28/02/2012 23:39, Graeme Russ a écrit :
> Hi Albert,
>
> On Wed, Feb 29, 2012 at 9:29 AM, Albert ARIBAUD
> <albert.u.boot at aribaud.net>  wrote:
>> Hi Alex,
>>
>> Le 21/02/2012 00:24, Alex Hornung a écrit :
>>>
>>> Hi,
>>>
>>> I've run into some memory corruption due to an error in the logic used
>>> to allocate the bd (and gd) during board_init of the nios2.
>>>
>>>
>>> #define CONFIG_SYS_GBL_DATA_OFFSET      (CONFIG_SYS_MALLOC_BASE - \
>>>                                           GENERATED_GBL_DATA_SIZE)
>>> [...]
>>>
>>>          gd = (gd_t *)CONFIG_SYS_GBL_DATA_OFFSET;
>>> [...]
>>>          gd->bd = (bd_t *)(gd+1);        /* At end of global data */
>>> [...]
>>>          mem_malloc_init(CONFIG_SYS_MALLOC_BASE, CONFIG_SYS_MALLOC_LEN);
>>>
>>> The relevant points here are that CONFIG_SYS_GBL_DATA_OFFSET is
>>> GENERATED_GBL_DATA_SIZE (80) bytes below the CONFIG_SYS_MALLOC_BASE.
>>>
>>> Given that gd is 68 bytes big, now the start of bd is only 12 bytes from
>>> the beginning of the malloc base - but the size of bd is 36 bytes!
>>
>>
>> So GENERATED_GBL_DATA_SIZE is wrong if it was supposed to contain both gd
>> and bd, which I suspect is not the case; but if it is supposed to only
>> contain a gd, then the definition of CONFIG_SYS_GBL_DATA_OFFSET is wrong in
>> that it does not account for gd and bd as it should.
>
> The global data struct only contains a pointer to the board data struct.
>
> IMHO I think the approach taken (but almost all arches) is very errror prone
> as it relies on manually laying out gd and bd in memory with bd sitting
> immediately above or below gd. In theory, this layout should never be
> tampered with, but I still don't like it.
>
> For x86, gd and bd are in BSS after relocation, so there is no need to
> hack around them when calculating the heap or stack, but I have a sneaking
> suspicion that this could make debugging harder as there is no way to
> reliably find the relocation offset as gd is never located at a known
> location in memory...

Duh. I had misread the code... Time for me to go to sleep. :/

For ARM we have gd in r8, which makes things simpler.

Anyway -- this does not affect the fact that GENERATED_GBL_DATA_SIZE 
should be equal to or greater than sizeof(gd_t)+sizeof(bd-t), right?

> Regards,
>
> Graeme

Amicalement,
-- 
Albert.