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