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

[Albert ARIBAUD]

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.

(BTW, what makes GENERATED_GBL_DATA_SIZE different from sizeof(gd_t)?)

> In other words, bd and the malloc base overlap, causing memory
> corruption in some of the malloc'd memory when some bd elements are
> populated. In my case in particular some content of the flash mtd
> eraseregions is getting corrupted by the write to bd->bi_ip_addr after
> initializing the flash stuff.
>
> I'm not sure how this should be dealt with - I'd think the best approach
> here is to change the CONFIG_SYS_GBL_DATA_OFFSET to include some space
> for the bd, or malloc'ing the bd.
>
> If you let me know which one is the preferred approach, I'll gladly
> provide a patch.

Hmm... You have sizeof(bd_t) available, so you could do something like

#define CONFIG_SYS_GBL_DATA_OFFSET      (CONFIG_SYS_MALLOC_BASE - \
                                             sizeof(bd_t) - \
 >                                           GENERATED_GBL_DATA_SIZE)

That would ensure you have space available for a gd and bd.

Amicalement,
-- 
Albert.