diff mbox series

arm64: Fix relocation of env_addr if POSITION_INDEPENDENT=y

Message ID 1623738782-10072-1-git-send-email-hayashi.kunihiko@socionext.com
State Accepted
Commit 534f0fbd65203871c2b054096422a5d0f3346cb1
Headers show
Series arm64: Fix relocation of env_addr if POSITION_INDEPENDENT=y | expand

Commit Message

Kunihiko Hayashi June 15, 2021, 6:33 a.m. UTC
If both POSITION_INDEPENDENT and SYS_RELOC_GD_ENV_ADDR are enabled,
wherever original env is placed anywhere, it should be relocated to
the right address.

Relocation offset gd->reloc_off is calculated with SYS_TEXT_BASE in
setup_reloc() and env address gd->env_addr is relocated by the offset in
initr_reloc_global_data().

gd->env_addr
  = (orig env) + gd->reloc_off
  = (orig env) + (gd->relocaddr - SYS_TEXT_BASE)

However, SYS_TEXT_BASE isn't always runtime base address when
POSITION_INDEPENDENT is enabled. So the relocated env_addr might point to
wrong address. For example, if SYS_TEXT_BASE is zero, gd->env_addr is
out of memory location and memory exception will occur.

There is a difference between linked address such as SYS_TEXT_BASE and
runtime base address. In _main, the difference is calculated as
"run-vs-link" offset. The env_addr should also be added to the offset
to fix the address.

gd->env_addr
  = (orig env) + ("run-vs-link" offset)   + gd->reloc_off
  = (orig env) + (SYS_TEXT_BASE - _start) + (gd->relocaddr - SYS_TEXT_BASE)
  = (orig env) + (gd->relocaddr - _start)

Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

---
 arch/arm/lib/crt0_64.S | 5 +++++
 lib/asm-offsets.c      | 2 ++
 2 files changed, 7 insertions(+)

This patch is based on the previous topic:
"env: Leave invalid env for nowhere location"
https://patchwork.ozlabs.org/project/uboot/patch/1620828554-24013-1-git-send-email-hayashi.kunihiko@socionext.com/

-- 
2.7.4

Comments

Marek Vasut June 27, 2021, 1:42 a.m. UTC | #1
On 6/15/21 8:33 AM, Kunihiko Hayashi wrote:
> If both POSITION_INDEPENDENT and SYS_RELOC_GD_ENV_ADDR are enabled,

> wherever original env is placed anywhere, it should be relocated to

> the right address.

> 

> Relocation offset gd->reloc_off is calculated with SYS_TEXT_BASE in

> setup_reloc() and env address gd->env_addr is relocated by the offset in

> initr_reloc_global_data().

> 

> gd->env_addr

>    = (orig env) + gd->reloc_off

>    = (orig env) + (gd->relocaddr - SYS_TEXT_BASE)

> 

> However, SYS_TEXT_BASE isn't always runtime base address when

> POSITION_INDEPENDENT is enabled. So the relocated env_addr might point to

> wrong address. For example, if SYS_TEXT_BASE is zero, gd->env_addr is

> out of memory location and memory exception will occur.

> 

> There is a difference between linked address such as SYS_TEXT_BASE and

> runtime base address. In _main, the difference is calculated as

> "run-vs-link" offset. The env_addr should also be added to the offset

> to fix the address.

> 

> gd->env_addr

>    = (orig env) + ("run-vs-link" offset)   + gd->reloc_off

>    = (orig env) + (SYS_TEXT_BASE - _start) + (gd->relocaddr - SYS_TEXT_BASE)

>    = (orig env) + (gd->relocaddr - _start)

> 

> Cc: Marek Vasut <marex@denx.de>

> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>


Thank you for debugging and fixing this properly.

Acked-by: Marek Vasut <marex@denx.de>

Tested-by: Marek Vasut <marex@denx.de>


I did manage to reproduce it on RCar3, and this patch fixes the crash on 
boot indeed.

Tom, it would be good to include it in this release too.
Tom Rini June 29, 2021, 12:38 p.m. UTC | #2
On Tue, Jun 15, 2021 at 03:33:02PM +0900, Kunihiko Hayashi wrote:

> If both POSITION_INDEPENDENT and SYS_RELOC_GD_ENV_ADDR are enabled,

> wherever original env is placed anywhere, it should be relocated to

> the right address.

> 

> Relocation offset gd->reloc_off is calculated with SYS_TEXT_BASE in

> setup_reloc() and env address gd->env_addr is relocated by the offset in

> initr_reloc_global_data().

> 

> gd->env_addr

>   = (orig env) + gd->reloc_off

>   = (orig env) + (gd->relocaddr - SYS_TEXT_BASE)

> 

> However, SYS_TEXT_BASE isn't always runtime base address when

> POSITION_INDEPENDENT is enabled. So the relocated env_addr might point to

> wrong address. For example, if SYS_TEXT_BASE is zero, gd->env_addr is

> out of memory location and memory exception will occur.

> 

> There is a difference between linked address such as SYS_TEXT_BASE and

> runtime base address. In _main, the difference is calculated as

> "run-vs-link" offset. The env_addr should also be added to the offset

> to fix the address.

> 

> gd->env_addr

>   = (orig env) + ("run-vs-link" offset)   + gd->reloc_off

>   = (orig env) + (SYS_TEXT_BASE - _start) + (gd->relocaddr - SYS_TEXT_BASE)

>   = (orig env) + (gd->relocaddr - _start)

> 

> Cc: Marek Vasut <marex@denx.de>

> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

> Acked-by: Marek Vasut <marex@denx.de>

> Tested-by: Marek Vasut <marex@denx.de>


Applied to u-boot/master, thanks!

-- 
Tom
diff mbox series

Patch

diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
index 9d2319c..680e674 100644
--- a/arch/arm/lib/crt0_64.S
+++ b/arch/arm/lib/crt0_64.S
@@ -112,6 +112,11 @@  ENTRY(_main)
 	ldr	x9, _TEXT_BASE		/* x9 <- Linked value of _start */
 	sub	x9, x9, x0		/* x9 <- Run-vs-link offset */
 	add	lr, lr, x9
+#if defined(CONFIG_SYS_RELOC_GD_ENV_ADDR)
+	ldr	x0, [x18, #GD_ENV_ADDR]	/* x0 <- gd->env_addr */
+	add	x0, x0, x9
+	str	x0, [x18, #GD_ENV_ADDR]
+#endif
 #endif
 	/* Add in link-vs-relocation offset */
 	ldr	x9, [x18, #GD_RELOC_OFF]	/* x9 <- gd->reloc_off */
diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c
index ee592cf..c691066 100644
--- a/lib/asm-offsets.c
+++ b/lib/asm-offsets.c
@@ -41,5 +41,7 @@  int main(void)
 
 	DEFINE(GD_NEW_GD, offsetof(struct global_data, new_gd));
 
+	DEFINE(GD_ENV_ADDR, offsetof(struct global_data, env_addr));
+
 	return 0;
 }