From mboxrd@z Thu Jan  1 00:00:00 1970
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
Date: Tue, 28 Feb 2012 23:29:36 +0100
Subject: [U-Boot] memory corruption on nios2 due to overlap of gbl data
 and malloc
In-Reply-To: <4F42D61C.6080201@alexhornung.com>
References: <4F42D61C.6080201@alexhornung.com>
Message-ID: <4F4D5550.7010701@aribaud.net>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.