[U-Boot] Image.c get_table_entry_name
The string pointers in uimage_{arch,os,type,comp}[] are not being relocated and still point to flash. If flash is erased (all f's), this causes bootm to trap on a bad pointer.
This is for powerpc (mpc8572ds). How are they suppose to be relocated?
-EdS
=> boot boot ## Booting kernel from Legacy Image at 01000000 ... Image Name: Linux-2.6.26-f8572-8572ds-00610- Image Type: Bad trap at PC: 3ffa7ae8, SR: 29200, vector=d00 NIP: 3FFA7AE8 XER: 20000000 LR: 3FFA74F8 REGS: 3fe9d8b0 TRAP: 0d00 DAR: F0000000 MSR: 00029200 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 00
GPR00: 000000FF 3FE9D9A0 3FE9DF70 F0000000 00000000 3FE9D9E0 EFFB8358 EFFB8300 GPR08: 000000A0 EFFB849C 3FFD7FD5 00000002 00000001 10030BD0 3FFEAC00 00000000 GPR16: 3FFC28D0 3FFD782C 00000000 00000000 00000000 00000000 00000000 3FFD6168 GPR24: 3FE9D9A8 3FE9D9EC 00000000 FFFFFFFF EFFB849C 3FE9D9EC 3FFEADCC 3FE9D9E0
Hi Ed,
The string pointers in uimage_{arch,os,type,comp}[] are not being relocated and still point to flash. If flash is erased (all f's), this causes bootm to trap on a bad pointer.
This is for powerpc (mpc8572ds). How are they suppose to be relocated?
-EdS
=> boot boot ## Booting kernel from Legacy Image at 01000000 ... Image Name: Linux-2.6.26-f8572-8572ds-00610- Image Type: Bad trap at PC: 3ffa7ae8, SR: 29200, vector=d00 NIP: 3FFA7AE8 XER: 20000000 LR: 3FFA74F8 REGS: 3fe9d8b0 TRAP: 0d00 DAR: F0000000 MSR: 00029200 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 00
GPR00: 000000FF 3FE9D9A0 3FE9DF70 F0000000 00000000 3FE9D9E0 EFFB8358 EFFB8300 GPR08: 000000A0 EFFB849C 3FFD7FD5 00000002 00000001 10030BD0 3FFEAC00 00000000 GPR16: 3FFC28D0 3FFD782C 00000000 00000000 00000000 00000000 00000000 3FFD6168 GPR24: 3FE9D9A8 3FE9D9EC 00000000 FFFFFFFF EFFB849C 3FE9D9EC 3FFEADCC 3FE9D9E0
Re-applying Grant's change that was backed out here: http://article.gmane.org/gmane.comp.boot-loaders.u-boot/33297 solved the same issue (with other commands) for me when using gcc 4.2.2 for an 8572-based board. eg:
- #define CONFIG_RELOC_FIXUP_WORKS in your board header - remove *(.fixup) from your board's linker script - add -mrelocatable to your compile flags
Best, Peter
Fixes boot crash from bad string pointers in get_table_entry_name when flash is erased or differs from current u-boot image.
Signed-off-by: Ed Swarthout Ed.Swarthout@freescale.com ---
Fix was pointed out by Peter Tyser in Image.c get_table_entry_name thread.
This redoes Grant Likey's relocation change, but leaves control to each board. This also fixes the "mii dump" command when flash is erased. Tested with gcc 4.2
board/freescale/mpc8572ds/config.mk | 1 + board/freescale/mpc8572ds/u-boot.lds | 2 +- 2 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/board/freescale/mpc8572ds/config.mk b/board/freescale/mpc8572ds/config.mk index 5b32186..8b3651b 100644 --- a/board/freescale/mpc8572ds/config.mk +++ b/board/freescale/mpc8572ds/config.mk @@ -30,3 +30,4 @@ endif PLATFORM_CPPFLAGS += -DCONFIG_E500=1 PLATFORM_CPPFLAGS += -DCONFIG_MPC85xx=1 PLATFORM_CPPFLAGS += -DCONFIG_MPC8572=1 +PLATFORM_CPPFLAGS += -mrelocatable -DCONFIG_RELOC_FIXUP_WORKS diff --git a/board/freescale/mpc8572ds/u-boot.lds b/board/freescale/mpc8572ds/u-boot.lds index a05ece5..79fb41f 100644 --- a/board/freescale/mpc8572ds/u-boot.lds +++ b/board/freescale/mpc8572ds/u-boot.lds @@ -58,7 +58,7 @@ SECTIONS .text : { *(.text) - *(.fixup) +/* *(.fixup)*/ *(.got1) } :text _etext = .;
Ed Swarthout wrote:
Fixes boot crash from bad string pointers in get_table_entry_name when flash is erased or differs from current u-boot image.
Signed-off-by: Ed Swarthout Ed.Swarthout@freescale.com
Fix was pointed out by Peter Tyser in Image.c get_table_entry_name thread.
This redoes Grant Likey's relocation change, but leaves control to each board. This also fixes the "mii dump" command when flash is erased. Tested with gcc 4.2
board/freescale/mpc8572ds/config.mk | 1 + board/freescale/mpc8572ds/u-boot.lds | 2 +- 2 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/board/freescale/mpc8572ds/config.mk b/board/freescale/mpc8572ds/config.mk index 5b32186..8b3651b 100644 --- a/board/freescale/mpc8572ds/config.mk +++ b/board/freescale/mpc8572ds/config.mk @@ -30,3 +30,4 @@ endif PLATFORM_CPPFLAGS += -DCONFIG_E500=1 PLATFORM_CPPFLAGS += -DCONFIG_MPC85xx=1 PLATFORM_CPPFLAGS += -DCONFIG_MPC8572=1 +PLATFORM_CPPFLAGS += -mrelocatable -DCONFIG_RELOC_FIXUP_WORKS diff --git a/board/freescale/mpc8572ds/u-boot.lds b/board/freescale/mpc8572ds/u-boot.lds index a05ece5..79fb41f 100644 --- a/board/freescale/mpc8572ds/u-boot.lds +++ b/board/freescale/mpc8572ds/u-boot.lds @@ -58,7 +58,7 @@ SECTIONS .text : { *(.text)
- *(.fixup)
+/* *(.fixup)*/
Please don't (just) comment this out.
Preferably your commit message would have enough information to answer the question of why .fixup was removed and no comment in the linker control file would be necessary.
In some cases it is worth adding a comment to the code as to why .fixup was removed so that someone that doesn't know the background doesn't add it back in. Something like this hand generated pseudo patch:
+/* + * Note: The *(.fixup) section is unnecessary because the + * CONFIG_RELOC_FIXUP_WORKS method is being used to do the relocation. + */
.text : { *(.text) - *(.fixup) *(.got1) } :text
Thanks, gvb
Dear Ed Swarthout,
In message 1220862412-16162-1-git-send-email-Ed.Swarthout@freescale.com you wrote:
Fixes boot crash from bad string pointers in get_table_entry_name when flash is erased or differs from current u-boot image.
Signed-off-by: Ed Swarthout Ed.Swarthout@freescale.com
Fix was pointed out by Peter Tyser in Image.c get_table_entry_name thread.
This redoes Grant Likey's relocation change, but leaves control to each board. This also fixes the "mii dump" command when flash is erased. Tested with gcc 4.2
NAK.
Sorry, this doesn't work. We cannot have one board compile this way, and another one another way - one working with that tool chain and the other with another tool chain only.
Theproblem you're addressing is a bug, and not only on this single board, but everywhere. So it must be fixed everywhere.
Either we add manual relocation to these pointers like we do elsewhere (that would be my recommendation for a quick workaround aka bug fix for this release), or we should find a clean way to get real relocation working (that would be much better, but probably it is too late for this release).
Best regards,
Wolfgang Denk
Wolfgang,
On Mon, Oct 13, 2008 at 9:53 AM, Wolfgang Denk wd@denx.de wrote:
Dear Ed Swarthout,
In message 1220862412-16162-1-git-send-email-Ed.Swarthout@freescale.com you wrote:
Fixes boot crash from bad string pointers in get_table_entry_name when flash is erased or differs from current u-boot image.
This is exactly the problem I am looking at for my board so I can do flash upgrades of U-Boot from U-Boot
Either we add manual relocation to these pointers like we do elsewhere (that would be my recommendation for a quick workaround aka bug fix for this release), or we should find a clean way to get real relocation working (that would be much better, but probably it is too late for this release).
I am looking into this this using gcc -fpie / ld -pie. This is used by hardened system for address space randomization. It appears to be very well supported by recent versions of gcc and binutils on all platforms so appears to me to be the way to go.
The number of new sections generated in a PIE image is more than I expected. The following sections have differences when TEXT_BASE is changed when compiled with -fpie and linked with -pie:
.dynsym .dynamic .data.rel .data.rel.local .data.rel.ro.local .got.plt .got .rel.got .rel.text .rel.u_boot_cmd .rel.data.rel .rel.data.rel.ro.local .rel.data.rel.local
Unfortunately, not all of these appear to be record-based sections, and may even cross-reference each other. I was looking at throwing this in the 'too hard' basket for now, keeping -fpie / -pie and just setting TEXT_BASE to a fixed address in RAM and look at implementing the relocator later. However, it seems to me that implementing full, clean relocation just keeps on popping up as a real issue.
Wolfgang - If you like, I could bump this up my priority list and get PIE support added to lib_i386 and from there it can be ported to other arches. Unfortunately my work to date has been for a custom i386 board so whatever I come up with may not tie 100% cleanly into mainline. I have been maintaining a detailed step-by-step commit path, so it should not be that bad, but expect ~20 commits from mainline to PIE.
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de The optimum committee has no members. - Norman Augustine _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Mon, 2008-10-13 at 00:53 +0200, Wolfgang Denk wrote:
Dear Ed Swarthout,
In message 1220862412-16162-1-git-send-email-Ed.Swarthout@freescale.com you wrote:
Fixes boot crash from bad string pointers in get_table_entry_name when flash is erased or differs from current u-boot image.
Signed-off-by: Ed Swarthout Ed.Swarthout@freescale.com
Fix was pointed out by Peter Tyser in Image.c get_table_entry_name thread.
This redoes Grant Likey's relocation change, but leaves control to each board. This also fixes the "mii dump" command when flash is erased. Tested with gcc 4.2
Just wanted to point out that the problems from Grants relocation change might come from something I found a while back, quoting from previous mail:
For fun I had a look into eabi.asm code at http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/eabi.asm?rev=1.1... and I noticed one difference: The __eabi_uconvert() function skips NULL ptrs.
Perhaps this is the missing piece needed in start.S for PPC?
__eabi_convert pasted below for convenience.
Jocke
FUNC_START(__eabi_convert) cmplw 1,3,4 /* any pointers to convert? */ subf 5,3,4 /* calculate number of words to convert */ bclr 4,4 /* return if no pointers */
srawi 5,5,2 addi 3,3,-4 /* start-4 for use with lwzu */ mtctr 5
.Lcvt: lwzu 6,4(3) /* pointer to convert */ cmpwi 0,6,0 beq- .Lcvt2 /* if pointer is null, don't convert */
add 6,6,12 /* convert pointer */ stw 6,0(3) .Lcvt2: bdnz+ .Lcvt blr
From: Joakim Tjernlund [mailto:joakim.tjernlund@transmode.se]
The __eabi_uconvert() function skips NULL ptrs.
Perhaps this is the missing piece needed in start.S for PPC?
I'm not seeing any zeros in the .got2 or .fixup entries in the .reloc section on 85xx when compiling with -mrelocatable.
So I don't think that would help.
-Ed
On Mon, 2008-10-13 at 20:59 -0700, Swarthout Edward L wrote:
From: Joakim Tjernlund [mailto:joakim.tjernlund@transmode.se]
The __eabi_uconvert() function skips NULL ptrs.
Perhaps this is the missing piece needed in start.S for PPC?
I'm not seeing any zeros in the .got2 or .fixup entries in the .reloc section on 85xx when compiling with -mrelocatable.
So I don't think that would help.
Neither do I but how do you know that others don't? It can depend on your u-boot config and/or gcc version. It is a bug anyway, it makes sense not to mess with NULL ptrs
Jocke
From: Wolfgang Denk [mailto:wd@denx.de]
NAK.
Sorry, this doesn't work. We cannot have one board compile this way, and another one another way - one working with that tool chain and the other with another tool chain only.
Is there a list of the toolchains that -mrelocatable must be tested against for each board?
The problem you're addressing is a bug, and not only on this single board, but everywhere. So it must be fixed everywhere.
Either we add manual relocation to these pointers like we do elsewhere (that would be my recommendation for a quick workaround aka bug fix for this release), or we should find a clean way to get real relocation working (that would be much better, but probably it is too late for this release).
I know of pointers in structures in cmd_mii.c and image.c that need relocation, but how many more are there? And where should the manual relocation code reside?
I think the effort should be applied to making sure the toolchain can work with relocatable instead.
In my testing of this patch, I found a crash with the Radeon driver because the x86emu code forces entries into got2 - which get double relocated when using -mrelocatedable.
See: [PATCH 1/2] Make mpc8572ds, mpc8544ds, mpc8536ds relocatable [PATCH 2/2] Leave x86emu op code tables in default section
Here: http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/48102
For a new version of this patch.
-Ed
participants (8)
-
Ed Swarthout -
Graeme Russ -
JerryVanBaren -
Joakim Tjernlund -
Peter Tyser -
Swarthout Edward L
-
Swarthout Edward L-SWARTHOU
-
Wolfgang Denk