cmd_fdt: save fdtaddr in hex format

PATCH New Sudeep Holla 3 participants 8 messages Jul 3, 2015 – Jul 20, 2015

Share & tools

Sudeep Holla

author

2015-07-03 17:29 UTC #0 lore raw

PATCH 1/1 cmd_fdt: save fdtaddr in hex format

Commit 90fbee3e4051 ("cmd_fdt: Actually fix fdt command in sandbox") changed the format(from hex address to unsigned long) in which "fdtaddr" is saved . However do_fdt continues reads the "fdtaddr" assuming it to be in hex format. This may lead to fdt being either loaded or attempted to load at erroneous address generating fault if the address is out of memory.

This patch changes back the format to hex while saving the "fdtaddr" as it was done before.

Cc Linus Walleij <[email protected]>

Cc Tom Rini <[email protected]>

Cc Simon Glass <[email protected]>

Cc Hua Yanghao <[email protected]>

Cc Heiko Schocher <[email protected]>

---
 common/cmd_fdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi,

You will see my signoff missing and it's intentional as I officially can't contribute to Uboot. I found this issue and it's been there for a while now, so thought of atleast reporting it(with a patch). So if anyone can review and submit this or any appropriate fix that would be great. I see this issue on vexpress TC2.

Regards,
Sudeep

common/cmd_fdt.c +1 -1

index 682b6553958f..4c18962d8532 100644
@@ -45,7 +45,7 @@ void set_working_fdt_addr(ulong addr)
 
 	buf = map_sysmem(addr, 0);
 	working_fdt = buf;
-	setenv_ulong("fdtaddr", addr);
+	setenv_hex("fdtaddr", addr);
 }
 
 /*

--
1.9.1

Simon Glass

2015-07-03 23:06 UTC #1 lore raw

PATCH 1/1 cmd_fdt: save fdtaddr in hex format

Hi Sudeep,

Quoted 19 lines · Sudeep Holla

Commit 90fbee3e4051 ("cmd_fdt: Actually fix fdt command in sandbox") changed the format(from hex address to unsigned long) in which "fdtaddr" is saved . However do_fdt continues reads the "fdtaddr" assuming it to be in hex format. This may lead to fdt being either loaded or attempted to load at erroneous address generating fault if the address is out of memory.
This patch changes back the format to hex while saving the "fdtaddr" as it was done before.
Cc Linus Walleij <[email protected]>
Cc Tom Rini <[email protected]>
Cc Simon Glass <[email protected]>
Cc Hua Yanghao <[email protected]>
Cc Heiko Schocher <[email protected]>
---
 common/cmd_fdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 7 lines

Hi,
You will see my signoff missing and it's intentional as I officially can't contribute to Uboot. I found this issue and it's been there for a while now, so thought of atleast reporting it(with a patch). So if anyone can review and submit this or any appropriate fix that would be great. I see this issue on vexpress TC2.

We can't take this without a sign-off - what stops you from adding it exactly?

The patch looks correct to me.

Regards,
Simon

Quoted 20 lines

Regards,
Sudeep

common/cmd_fdt.c +1 -2

index 682b6553958f..4c18962d8532 100644
@@ -45,7 +45,7 @@ void set_working_fdt_addr(ulong addr)
 
        buf = map_sysmem(addr, 0);
        working_fdt = buf;
-       setenv_ulong("fdtaddr", addr);
+       setenv_hex("fdtaddr", addr);
 }
 
 /*
--

1.9.1

Sudeep Holla

author

2015-07-06 08:47 UTC #2 lore raw

PATCH 1/1 cmd_fdt: save fdtaddr in hex format

Hi Simon,

Quoted 24 lines · Simon Glass

Hi Sudeep,
Quoted 21 lines · Sudeep Holla
Commit 90fbee3e4051 ("cmd_fdt: Actually fix fdt command in sandbox") changed the format(from hex address to unsigned long) in which "fdtaddr" is saved . However do_fdt continues reads the "fdtaddr" assuming it to be in hex format. This may lead to fdt being either loaded or attempted to load at erroneous address generating fault if the address is out of memory.
This patch changes back the format to hex while saving the "fdtaddr" as it was done before.
Cc Linus Walleij <[email protected]>
Cc Tom Rini <[email protected]>
Cc Simon Glass <[email protected]>
Cc Hua Yanghao <[email protected]>
Cc Heiko Schocher <[email protected]>
---
  common/cmd_fdt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
Hi,

Quoted 8 lines

Quoted 5 lines
You will see my signoff missing and it's intentional as I officially can't contribute to Uboot. I found this issue and it's been there for a while now, so thought of atleast reporting it(with a patch). So if anyone can review and submit this or any appropriate fix that would be great. I see this issue on vexpress TC2.
We can't take this without a sign-off - what stops you from adding it exactly?

Unfortunately it's not in the list of legally approved projects I can contribute. So I am bound not to do so legally.

Hi Linus,

Since you did a lot work on vexpress/Juno quite recently on ARM's request, can you cook up a similar patch to fix the above mentioned issue if that's OK for you ?

Regards,
Sudeep

Sudeep Holla

author

2015-07-10 16:18 UTC #3 lore raw

PATCH 1/1 cmd_fdt: save fdtaddr in hex format

In reply to Sudeep Holla

This patch changes back the format to hex while saving the "fdtaddr" as it was done before.

Signed-off-by Sudeep Holla <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Tom Rini <[email protected]>

Cc Simon Glass <[email protected]>

Cc Hua Yanghao <[email protected]>

Cc Heiko Schocher <[email protected]>

---
 common/cmd_fdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Reposting this time with my signoff as it turns out I can legally do so.

common/cmd_fdt.c +1 -1

index 682b6553958f..4c18962d8532 100644
@@ -45,7 +45,7 @@ void set_working_fdt_addr(ulong addr)
 
 	buf = map_sysmem(addr, 0);
 	working_fdt = buf;
-	setenv_ulong("fdtaddr", addr);
+	setenv_hex("fdtaddr", addr);
 }
 
 /*

Signature · 1 line

1.9.1

Simon Glass

2015-07-10 23:08 UTC #4 lore raw

PATCH 1/1 cmd_fdt: save fdtaddr in hex format

Quoted 17 lines · Sudeep Holla

Commit 90fbee3e4051 ("cmd_fdt: Actually fix fdt command in sandbox") changed the format(from hex address to unsigned long) in which "fdtaddr" is saved . However do_fdt continues reads the "fdtaddr" assuming it to be in hex format. This may lead to fdt being either loaded or attempted to load at erroneous address generating fault if the address is out of memory.
This patch changes back the format to hex while saving the "fdtaddr" as it was done before.
Signed-off-by Sudeep Holla <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Tom Rini <[email protected]>
Cc Simon Glass <[email protected]>
Cc Hua Yanghao <[email protected]>
Cc Heiko Schocher <[email protected]>

Quoted 6 lines

---
 common/cmd_fdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reposting this time with my signoff as it turns out I can legally do so.

Great!

- Simon

Linus Walleij

2015-07-16 13:26 UTC #5 lore raw

PATCH 1/1 cmd_fdt: save fdtaddr in hex format

In reply to Sudeep Holla

Quoted 10 lines · Sudeep Holla

Commit 90fbee3e4051 ("cmd_fdt: Actually fix fdt command in sandbox") changed the format(from hex address to unsigned long) in which "fdtaddr" is saved . However do_fdt continues reads the "fdtaddr" assuming it to be in hex format. This may lead to fdt being either loaded or attempted to load at erroneous address generating fault if the address is out of memory.
This patch changes back the format to hex while saving the "fdtaddr" as it was done before.

Quoted 6 lines

Signed-off-by Sudeep Holla <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Tom Rini <[email protected]>
Cc Simon Glass <[email protected]>
Cc Hua Yanghao <[email protected]>
Cc Heiko Schocher <[email protected]>

Looks like the right fix:

Acked-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

Simon Glass

2015-07-16 16:34 UTC #6 lore raw

PATCH 1/1 cmd_fdt: save fdtaddr in hex format

Quoted 19 lines · Linus Walleij

Quoted 10 lines · Sudeep Holla
Commit 90fbee3e4051 ("cmd_fdt: Actually fix fdt command in sandbox") changed the format(from hex address to unsigned long) in which "fdtaddr" is saved . However do_fdt continues reads the "fdtaddr" assuming it to be in hex format. This may lead to fdt being either loaded or attempted to load at erroneous address generating fault if the address is out of memory.
This patch changes back the format to hex while saving the "fdtaddr" as it was done before.
Quoted 6 lines
Signed-off-by Sudeep Holla <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Tom Rini <[email protected]>
Cc Simon Glass <[email protected]>
Cc Hua Yanghao <[email protected]>
Cc Heiko Schocher <[email protected]>

Quoted 5 lines

Looks like the right fix:
Acked-by Linus Walleij <[email protected]>
Yours,
Linus Walleij

Acked-by Simon Glass <[email protected]>

Simon Glass

2015-07-20 13:25 UTC #7 lore raw

PATCH 1/1 cmd_fdt: save fdtaddr in hex format

Quoted 20 lines · Simon Glass

Quoted 19 lines · Linus Walleij
Quoted 16 lines · Sudeep Holla
Commit 90fbee3e4051 ("cmd_fdt: Actually fix fdt command in sandbox") changed the format(from hex address to unsigned long) in which "fdtaddr" is saved . However do_fdt continues reads the "fdtaddr" assuming it to be in hex format. This may lead to fdt being either loaded or attempted to load at erroneous address generating fault if the address is out of memory.
This patch changes back the format to hex while saving the "fdtaddr" as it was done before.
Signed-off-by Sudeep Holla <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Tom Rini <[email protected]>
Cc Simon Glass <[email protected]>
Cc Hua Yanghao <[email protected]>
Cc Heiko Schocher <[email protected]>

Quoted 7 lines

Quoted 5 lines
Looks like the right fix:
Acked-by Linus Walleij <[email protected]>
Yours,
Linus Walleij
Acked-by Simon Glass <[email protected]>

Applied to u-boot-fdt, thanks!