* [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index
@ 2019-04-24 11:49 Keerthy
2019-04-24 23:59 ` Simon Glass
2019-07-09 19:38 ` Eugeniu Rosca
0 siblings, 2 replies; 9+ messages in thread
From: Keerthy @ 2019-04-24 11:49 UTC (permalink / raw)
To: u-boot
Add ofnode_get_addr_size_index function to fetch the address
and size of the reg space based on index.
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
Previous discussion can be found here:
* https://patchwork.ozlabs.org/patch/999659/
drivers/core/ofnode.c | 13 ++++++++++---
include/dm/ofnode.h | 14 ++++++++++++++
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 785f5c3acf..5bb06d3b00 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -251,7 +251,7 @@ int ofnode_read_size(ofnode node, const char *propname)
return -EINVAL;
}
-fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
+fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size)
{
int na, ns;
@@ -260,7 +260,7 @@ fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
uint flags;
prop_val = of_get_address(ofnode_to_np(node), index,
- NULL, &flags);
+ (u64 *)size, &flags);
if (!prop_val)
return FDT_ADDR_T_NONE;
@@ -277,12 +277,19 @@ fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
ns = ofnode_read_simple_size_cells(ofnode_get_parent(node));
return fdtdec_get_addr_size_fixed(gd->fdt_blob,
ofnode_to_offset(node), "reg",
- index, na, ns, NULL, true);
+ index, na, ns, size, true);
}
return FDT_ADDR_T_NONE;
}
+fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
+{
+ fdt_size_t size;
+
+ return ofnode_get_addr_size_index(node, index, &size);
+}
+
fdt_addr_t ofnode_get_addr(ofnode node)
{
return ofnode_get_addr_index(node, 0);
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index d206ee2caa..1be5ba4b45 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -354,6 +354,20 @@ ofnode ofnode_get_by_phandle(uint phandle);
*/
int ofnode_read_size(ofnode node, const char *propname);
+/**
+ * ofnode_get_addr_size_index() - get an address/size from a node
+ * based on index
+ *
+ * This reads the register address/size from a node based on index
+ *
+ * @node: node to read from
+ * @index: Index of address to read (0 for first)
+ * @size: Pointer to size of the address
+ * @return address, or FDT_ADDR_T_NONE if not present or invalid
+ */
+phys_addr_t ofnode_get_addr_size_index(ofnode node, int index,
+ fdt_size_t *size);
+
/**
* ofnode_get_addr_index() - get an address from a node
*
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index
2019-04-24 11:49 [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index Keerthy
@ 2019-04-24 23:59 ` Simon Glass
2019-05-22 13:21 ` Simon Glass
2019-07-09 19:38 ` Eugeniu Rosca
1 sibling, 1 reply; 9+ messages in thread
From: Simon Glass @ 2019-04-24 23:59 UTC (permalink / raw)
To: u-boot
On Wed, 24 Apr 2019 at 05:49, Keerthy <j-keerthy@ti.com> wrote:
>
> Add ofnode_get_addr_size_index function to fetch the address
> and size of the reg space based on index.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>
> Previous discussion can be found here:
>
> * https://patchwork.ozlabs.org/patch/999659/
>
> drivers/core/ofnode.c | 13 ++++++++++---
> include/dm/ofnode.h | 14 ++++++++++++++
> 2 files changed, 24 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index
2019-04-24 23:59 ` Simon Glass
@ 2019-05-22 13:21 ` Simon Glass
0 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2019-05-22 13:21 UTC (permalink / raw)
To: u-boot
On Wed, 24 Apr 2019 at 17:59, Simon Glass <sjg@chromium.org> wrote:
>
> On Wed, 24 Apr 2019 at 05:49, Keerthy <j-keerthy@ti.com> wrote:
> >
> > Add ofnode_get_addr_size_index function to fetch the address
> > and size of the reg space based on index.
> >
> > Signed-off-by: Keerthy <j-keerthy@ti.com>
> > ---
> >
> > Previous discussion can be found here:
> >
> > * https://patchwork.ozlabs.org/patch/999659/
> >
> > drivers/core/ofnode.c | 13 ++++++++++---
> > include/dm/ofnode.h | 14 ++++++++++++++
> > 2 files changed, 24 insertions(+), 3 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index
2019-04-24 11:49 [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index Keerthy
2019-04-24 23:59 ` Simon Glass
@ 2019-07-09 19:38 ` Eugeniu Rosca
2019-08-13 9:33 ` Simon Glass
1 sibling, 1 reply; 9+ messages in thread
From: Eugeniu Rosca @ 2019-07-09 19:38 UTC (permalink / raw)
To: u-boot
Hi All,
On Wed, Apr 24, 2019 at 05:19:53PM +0530, Keerthy wrote:
> Add ofnode_get_addr_size_index function to fetch the address
> and size of the reg space based on index.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>
> Previous discussion can be found here:
>
> * https://patchwork.ozlabs.org/patch/999659/
>
> drivers/core/ofnode.c | 13 ++++++++++---
> include/dm/ofnode.h | 14 ++++++++++++++
> 2 files changed, 24 insertions(+), 3 deletions(-)
jFYI/FWIW, this patch, present in U-Boot/master as commit [1], revives
the issue fixed in [2]. More precisely, when compiling the sandbox
U-Boot with -fsanitize=address and linking it against -lasan,
the following backtrace pops up during boot [3].
[1] https://gitlab.denx.de/u-boot/u-boot/commit/e679d03b08fb
("core: ofnode: Add ofnode_get_addr_size_index")
[2] https://gitlab.denx.de/u-boot/u-boot/commit/9bfacf249b10
("core: ofnode: Fix ASAN-reported stack-buffer-overflow in of_get_address")
[3] The same issue as fixed in [2], now reproduced on v2019.07
==5996==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc65f869a0 at pc 0x564834c6cd89 bp 0x7ffc65f867b0 sp 0x7ffc65f867a0
WRITE of size 8 at 0x7ffc65f869a0 thread T0
#0 0x564834c6cd88 in of_get_address drivers/core/of_addr.c:154
#1 0x564834c74fac in ofnode_get_addr_size_index drivers/core/ofnode.c:262
#2 0x564834c75232 in ofnode_get_addr_index drivers/core/ofnode.c:290
#3 0x564834d35a4f in sb_eth_ofdata_to_platdata drivers/net/sandbox.c:422
#4 0x564834c5ad22 in device_probe drivers/core/device.c:407
#5 0x564834f2f1c4 in eth_initialize net/eth-uclass.c:399
#6 0x564834bf9c34 in initr_net common/board_r.c:565
#7 0x564834bfa580 in initcall_run_list include/initcall.h:38
#8 0x564834bfa580 in board_init_r common/board_r.c:867
#9 0x564834b8156d in main arch/sandbox/cpu/start.c:362
#10 0x7f2fe7912b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
#11 0x564834b85b29 in _start (/srv/R/u-boot-master/u-boot+0x792b29)
Address 0x7ffc65f869a0 is located in stack of thread T0 at offset 32 in frame
#0 0x564834c751c4 in ofnode_get_addr_index drivers/core/ofnode.c:287
This frame has 1 object(s):
[32, 36) 'size' (line 288) <== Memory access@offset 32 partially overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow drivers/core/of_addr.c:154 in of_get_address
Shadow bytes around the buggy address:
0x10000cbe8ce0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000cbe8cf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000cbe8d00: 00 00 f1 f1 f1 f1 04 f2 04 f2 04 f3 f3 f3 00 00
0x10000cbe8d10: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f1 f1
0x10000cbe8d20: 04 f2 00 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
=>0x10000cbe8d30: f1 f1 f1 f1[04]f3 f3 f3 00 00 00 00 00 00 00 00
0x10000cbe8d40: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00
0x10000cbe8d50: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000cbe8d60: f1 f1 f1 f1 00 f3 f3 f3 00 00 00 00 00 00 00 00
0x10000cbe8d70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000cbe8d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==5996==ABORTING
--
Best Regards,
Eugeniu.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index
2019-07-09 19:38 ` Eugeniu Rosca
@ 2019-08-13 9:33 ` Simon Glass
2019-08-13 9:55 ` Keerthy
0 siblings, 1 reply; 9+ messages in thread
From: Simon Glass @ 2019-08-13 9:33 UTC (permalink / raw)
To: u-boot
Hi Keethy,
On Tue, 9 Jul 2019 at 13:38, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
>
> Hi All,
>
> On Wed, Apr 24, 2019 at 05:19:53PM +0530, Keerthy wrote:
> > Add ofnode_get_addr_size_index function to fetch the address
> > and size of the reg space based on index.
> >
> > Signed-off-by: Keerthy <j-keerthy@ti.com>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Previous discussion can be found here:
> >
> > * https://patchwork.ozlabs.org/patch/999659/
> >
> > drivers/core/ofnode.c | 13 ++++++++++---
> > include/dm/ofnode.h | 14 ++++++++++++++
> > 2 files changed, 24 insertions(+), 3 deletions(-)
>
> jFYI/FWIW, this patch, present in U-Boot/master as commit [1], revives
> the issue fixed in [2]. More precisely, when compiling the sandbox
> U-Boot with -fsanitize=address and linking it against -lasan,
> the following backtrace pops up during boot [3].
>
> [1] https://gitlab.denx.de/u-boot/u-boot/commit/e679d03b08fb
> ("core: ofnode: Add ofnode_get_addr_size_index")
> [2] https://gitlab.denx.de/u-boot/u-boot/commit/9bfacf249b10
> ("core: ofnode: Fix ASAN-reported stack-buffer-overflow in of_get_address")
> [3] The same issue as fixed in [2], now reproduced on v2019.07
>
> ==5996==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc65f869a0 at pc 0x564834c6cd89 bp 0x7ffc65f867b0 sp 0x7ffc65f867a0
> WRITE of size 8 at 0x7ffc65f869a0 thread T0
> #0 0x564834c6cd88 in of_get_address drivers/core/of_addr.c:154
> #1 0x564834c74fac in ofnode_get_addr_size_index drivers/core/ofnode.c:262
> #2 0x564834c75232 in ofnode_get_addr_index drivers/core/ofnode.c:290
> #3 0x564834d35a4f in sb_eth_ofdata_to_platdata drivers/net/sandbox.c:422
> #4 0x564834c5ad22 in device_probe drivers/core/device.c:407
> #5 0x564834f2f1c4 in eth_initialize net/eth-uclass.c:399
> #6 0x564834bf9c34 in initr_net common/board_r.c:565
> #7 0x564834bfa580 in initcall_run_list include/initcall.h:38
> #8 0x564834bfa580 in board_init_r common/board_r.c:867
> #9 0x564834b8156d in main arch/sandbox/cpu/start.c:362
> #10 0x7f2fe7912b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
> #11 0x564834b85b29 in _start (/srv/R/u-boot-master/u-boot+0x792b29)
>
> Address 0x7ffc65f869a0 is located in stack of thread T0 at offset 32 in frame
> #0 0x564834c751c4 in ofnode_get_addr_index drivers/core/ofnode.c:287
>
> This frame has 1 object(s):
> [32, 36) 'size' (line 288) <== Memory access@offset 32 partially overflows this variable
> HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
> (longjmp and C++ exceptions *are* supported)
> SUMMARY: AddressSanitizer: stack-buffer-overflow drivers/core/of_addr.c:154 in of_get_address
> Shadow bytes around the buggy address:
> 0x10000cbe8ce0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x10000cbe8cf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x10000cbe8d00: 00 00 f1 f1 f1 f1 04 f2 04 f2 04 f3 f3 f3 00 00
> 0x10000cbe8d10: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f1 f1
> 0x10000cbe8d20: 04 f2 00 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
> =>0x10000cbe8d30: f1 f1 f1 f1[04]f3 f3 f3 00 00 00 00 00 00 00 00
> 0x10000cbe8d40: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00
> 0x10000cbe8d50: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x10000cbe8d60: f1 f1 f1 f1 00 f3 f3 f3 00 00 00 00 00 00 00 00
> 0x10000cbe8d70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x10000cbe8d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> Shadow gap: cc
> ==5996==ABORTING
>
Can you please take a look?
Regards,
Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index
2019-08-13 9:33 ` Simon Glass
@ 2019-08-13 9:55 ` Keerthy
2019-08-14 8:23 ` Keerthy
0 siblings, 1 reply; 9+ messages in thread
From: Keerthy @ 2019-08-13 9:55 UTC (permalink / raw)
To: u-boot
On 13/08/19 3:03 PM, Simon Glass wrote:
> Hi Keethy,
>
> On Tue, 9 Jul 2019 at 13:38, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
>>
>> Hi All,
>>
>> On Wed, Apr 24, 2019 at 05:19:53PM +0530, Keerthy wrote:
>>> Add ofnode_get_addr_size_index function to fetch the address
>>> and size of the reg space based on index.
>>>
>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> Previous discussion can be found here:
>>>
>>> * https://patchwork.ozlabs.org/patch/999659/
>>>
>>> drivers/core/ofnode.c | 13 ++++++++++---
>>> include/dm/ofnode.h | 14 ++++++++++++++
>>> 2 files changed, 24 insertions(+), 3 deletions(-)
>>
>> jFYI/FWIW, this patch, present in U-Boot/master as commit [1], revives
>> the issue fixed in [2]. More precisely, when compiling the sandbox
>> U-Boot with -fsanitize=address and linking it against -lasan,
>> the following backtrace pops up during boot [3].
>>
>> [1] https://gitlab.denx.de/u-boot/u-boot/commit/e679d03b08fb
>> ("core: ofnode: Add ofnode_get_addr_size_index")
>> [2] https://gitlab.denx.de/u-boot/u-boot/commit/9bfacf249b10
>> ("core: ofnode: Fix ASAN-reported stack-buffer-overflow in of_get_address")
>> [3] The same issue as fixed in [2], now reproduced on v2019.07
>>
>> ==5996==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc65f869a0 at pc 0x564834c6cd89 bp 0x7ffc65f867b0 sp 0x7ffc65f867a0
>> WRITE of size 8 at 0x7ffc65f869a0 thread T0
>> #0 0x564834c6cd88 in of_get_address drivers/core/of_addr.c:154
>> #1 0x564834c74fac in ofnode_get_addr_size_index drivers/core/ofnode.c:262
>> #2 0x564834c75232 in ofnode_get_addr_index drivers/core/ofnode.c:290
>> #3 0x564834d35a4f in sb_eth_ofdata_to_platdata drivers/net/sandbox.c:422
>> #4 0x564834c5ad22 in device_probe drivers/core/device.c:407
>> #5 0x564834f2f1c4 in eth_initialize net/eth-uclass.c:399
>> #6 0x564834bf9c34 in initr_net common/board_r.c:565
>> #7 0x564834bfa580 in initcall_run_list include/initcall.h:38
>> #8 0x564834bfa580 in board_init_r common/board_r.c:867
>> #9 0x564834b8156d in main arch/sandbox/cpu/start.c:362
>> #10 0x7f2fe7912b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
>> #11 0x564834b85b29 in _start (/srv/R/u-boot-master/u-boot+0x792b29)
>>
>> Address 0x7ffc65f869a0 is located in stack of thread T0 at offset 32 in frame
>> #0 0x564834c751c4 in ofnode_get_addr_index drivers/core/ofnode.c:287
>>
>> This frame has 1 object(s):
>> [32, 36) 'size' (line 288) <== Memory access@offset 32 partially overflows this variable
>> HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
>> (longjmp and C++ exceptions *are* supported)
>> SUMMARY: AddressSanitizer: stack-buffer-overflow drivers/core/of_addr.c:154 in of_get_address
>> Shadow bytes around the buggy address:
>> 0x10000cbe8ce0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x10000cbe8cf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x10000cbe8d00: 00 00 f1 f1 f1 f1 04 f2 04 f2 04 f3 f3 f3 00 00
>> 0x10000cbe8d10: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f1 f1
>> 0x10000cbe8d20: 04 f2 00 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
>> =>0x10000cbe8d30: f1 f1 f1 f1[04]f3 f3 f3 00 00 00 00 00 00 00 00
>> 0x10000cbe8d40: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00
>> 0x10000cbe8d50: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x10000cbe8d60: f1 f1 f1 f1 00 f3 f3 f3 00 00 00 00 00 00 00 00
>> 0x10000cbe8d70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x10000cbe8d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> Shadow byte legend (one shadow byte represents 8 application bytes):
>> Addressable: 00
>> Partially addressable: 01 02 03 04 05 06 07
>> Heap left redzone: fa
>> Freed heap region: fd
>> Stack left redzone: f1
>> Stack mid redzone: f2
>> Stack right redzone: f3
>> Stack after return: f5
>> Stack use after scope: f8
>> Global redzone: f9
>> Global init order: f6
>> Poisoned by user: f7
>> Container overflow: fc
>> Array cookie: ac
>> Intra object redzone: bb
>> ASan internal: fe
>> Left alloca redzone: ca
>> Right alloca redzone: cb
>> Shadow gap: cc
>> ==5996==ABORTING
>>
>
> Can you please take a look?
Sure.
>
> Regards,
> Simon
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index
2019-08-13 9:55 ` Keerthy
@ 2019-08-14 8:23 ` Keerthy
2019-08-14 9:47 ` Eugeniu Rosca
0 siblings, 1 reply; 9+ messages in thread
From: Keerthy @ 2019-08-14 8:23 UTC (permalink / raw)
To: u-boot
On 13/08/19 3:25 PM, Keerthy wrote:
>
>
> On 13/08/19 3:03 PM, Simon Glass wrote:
>> Hi Keethy,
>>
>> On Tue, 9 Jul 2019 at 13:38, Eugeniu Rosca <roscaeugeniu@gmail.com>
>> wrote:
>>>
>>> Hi All,
>>>
>>> On Wed, Apr 24, 2019 at 05:19:53PM +0530, Keerthy wrote:
>>>> Add ofnode_get_addr_size_index function to fetch the address
>>>> and size of the reg space based on index.
>>>>
>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>
>>>> Previous discussion can be found here:
>>>>
>>>> * https://patchwork.ozlabs.org/patch/999659/
>>>>
>>>> drivers/core/ofnode.c | 13 ++++++++++---
>>>> include/dm/ofnode.h | 14 ++++++++++++++
>>>> 2 files changed, 24 insertions(+), 3 deletions(-)
>>>
>>> jFYI/FWIW, this patch, present in U-Boot/master as commit [1], revives
>>> the issue fixed in [2]. More precisely, when compiling the sandbox
>>> U-Boot with -fsanitize=address and linking it against -lasan,
>>> the following backtrace pops up during boot [3].
>>>
>>> [1] https://gitlab.denx.de/u-boot/u-boot/commit/e679d03b08fb
>>> ("core: ofnode: Add ofnode_get_addr_size_index")
>>> [2] https://gitlab.denx.de/u-boot/u-boot/commit/9bfacf249b10
>>> ("core: ofnode: Fix ASAN-reported stack-buffer-overflow in
>>> of_get_address")
>>> [3] The same issue as fixed in [2], now reproduced on v2019.07
>>>
>>> ==5996==ERROR: AddressSanitizer: stack-buffer-overflow on address
>>> 0x7ffc65f869a0 at pc 0x564834c6cd89 bp 0x7ffc65f867b0 sp 0x7ffc65f867a0
>>> WRITE of size 8 at 0x7ffc65f869a0 thread T0
>>> #0 0x564834c6cd88 in of_get_address drivers/core/of_addr.c:154
>>> #1 0x564834c74fac in ofnode_get_addr_size_index
>>> drivers/core/ofnode.c:262
>>> #2 0x564834c75232 in ofnode_get_addr_index
>>> drivers/core/ofnode.c:290
>>> #3 0x564834d35a4f in sb_eth_ofdata_to_platdata
>>> drivers/net/sandbox.c:422
>>> #4 0x564834c5ad22 in device_probe drivers/core/device.c:407
>>> #5 0x564834f2f1c4 in eth_initialize net/eth-uclass.c:399
>>> #6 0x564834bf9c34 in initr_net common/board_r.c:565
>>> #7 0x564834bfa580 in initcall_run_list include/initcall.h:38
>>> #8 0x564834bfa580 in board_init_r common/board_r.c:867
>>> #9 0x564834b8156d in main arch/sandbox/cpu/start.c:362
>>> #10 0x7f2fe7912b6a in __libc_start_main
>>> (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
>>> #11 0x564834b85b29 in _start (/srv/R/u-boot-master/u-boot+0x792b29)
>>>
>>> Address 0x7ffc65f869a0 is located in stack of thread T0 at offset 32
>>> in frame
>>> #0 0x564834c751c4 in ofnode_get_addr_index
>>> drivers/core/ofnode.c:287
>>>
>>> This frame has 1 object(s):
>>> [32, 36) 'size' (line 288) <== Memory access@offset 32
>>> partially overflows this variable
>>> HINT: this may be a false positive if your program uses some custom
>>> stack unwind mechanism, swapcontext or vfork
>>> (longjmp and C++ exceptions *are* supported)
>>> SUMMARY: AddressSanitizer: stack-buffer-overflow
>>> drivers/core/of_addr.c:154 in of_get_address
>>> Shadow bytes around the buggy address:
>>> 0x10000cbe8ce0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 0x10000cbe8cf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 0x10000cbe8d00: 00 00 f1 f1 f1 f1 04 f2 04 f2 04 f3 f3 f3 00 00
>>> 0x10000cbe8d10: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f1 f1
>>> 0x10000cbe8d20: 04 f2 00 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
>>> =>0x10000cbe8d30: f1 f1 f1 f1[04]f3 f3 f3 00 00 00 00 00 00 00 00
>>> 0x10000cbe8d40: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00
>>> 0x10000cbe8d50: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 0x10000cbe8d60: f1 f1 f1 f1 00 f3 f3 f3 00 00 00 00 00 00 00 00
>>> 0x10000cbe8d70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 0x10000cbe8d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> Shadow byte legend (one shadow byte represents 8 application bytes):
>>> Addressable: 00
>>> Partially addressable: 01 02 03 04 05 06 07
>>> Heap left redzone: fa
>>> Freed heap region: fd
>>> Stack left redzone: f1
>>> Stack mid redzone: f2
>>> Stack right redzone: f3
>>> Stack after return: f5
>>> Stack use after scope: f8
>>> Global redzone: f9
>>> Global init order: f6
>>> Poisoned by user: f7
>>> Container overflow: fc
>>> Array cookie: ac
>>> Intra object redzone: bb
>>> ASan internal: fe
>>> Left alloca redzone: ca
>>> Right alloca redzone: cb
>>> Shadow gap: cc
>>> ==5996==ABORTING
>>>
>>
>> Can you please take a look?
>
> Sure.
Eugeniu,
Could you kindly help me with the steps to reproduce the issue?
I am sending out a fix patch to convert all the size instances to
fdt_size_t type.
I have posted: https://patchwork.ozlabs.org/patch/1146830/
I believe the above should fix the size mismatch.
Regards,
Keerthy
>
>>
>> Regards,
>> Simon
>>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index
2019-08-14 8:23 ` Keerthy
@ 2019-08-14 9:47 ` Eugeniu Rosca
2019-08-14 10:18 ` Keerthy
0 siblings, 1 reply; 9+ messages in thread
From: Eugeniu Rosca @ 2019-08-14 9:47 UTC (permalink / raw)
To: u-boot
Hi Keerthy,
On Wed, Aug 14, 2019 at 01:53:50PM +0530, Keerthy wrote:
> Eugeniu,
>
> Could you kindly help me with the steps to reproduce the issue?
Sure. My pleasure. See [0].
> I am sending out a fix patch to convert all the size instances to fdt_size_t
> type.
>
> I have posted: https://patchwork.ozlabs.org/patch/1146830/
>
> I believe the above should fix the size mismatch.
The patch appears to fix the problem. Thanks!
[0] Steps to reproduce the issue reported in
https://patchwork.ozlabs.org/patch/1090094/#2212555
* Apply patch [1] to u-boot/master
* u-boot (master) gcc --version
gcc (Ubuntu 7.4.0-1ubuntu1~16.04~ppa1) 7.4.0
* u-boot (master) make defconfig all -j12
* u-boot (master) ./u-boot -d arch/sandbox/dts/test.dtb
[1] Dirty patch to enable ASAN in sandbox U-Boot (v2019.10-rc2):
diff --git a/Makefile b/Makefile
index 3b0864ae8eaa..b8ef4f267ed5 100644
--- a/Makefile
+++ b/Makefile
@@ -391,7 +391,7 @@ KBUILD_CPPFLAGS := -D__KERNEL__ -D__UBOOT__
KBUILD_CFLAGS := -Wall -Wstrict-prototypes \
-Wno-format-security \
- -fno-builtin -ffreestanding $(CSTD_FLAG)
+ -fno-builtin -ffreestanding $(CSTD_FLAG) -fsanitize=address
KBUILD_CFLAGS += -fshort-wchar -fno-strict-aliasing
KBUILD_AFLAGS := -D__ASSEMBLY__
@@ -768,6 +768,7 @@ PLATFORM_LIBGCC = arch/$(ARCH)/lib/lib.a
else
PLATFORM_LIBGCC := -L $(shell dirname `$(CC) $(c_flags) -print-libgcc-file-name`) -lgcc
endif
+PLATFORM_LIBS := -lasan $(PLATFORM_LIBS)
PLATFORM_LIBS += $(PLATFORM_LIBGCC)
ifdef CONFIG_CC_COVERAGE
--
Best Regards,
Eugeniu.
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index
2019-08-14 9:47 ` Eugeniu Rosca
@ 2019-08-14 10:18 ` Keerthy
0 siblings, 0 replies; 9+ messages in thread
From: Keerthy @ 2019-08-14 10:18 UTC (permalink / raw)
To: u-boot
On 14/08/19 3:17 PM, Eugeniu Rosca wrote:
> Hi Keerthy,
>
> On Wed, Aug 14, 2019 at 01:53:50PM +0530, Keerthy wrote:
>> Eugeniu,
>>
>> Could you kindly help me with the steps to reproduce the issue?
>
> Sure. My pleasure. See [0].
>
>> I am sending out a fix patch to convert all the size instances to fdt_size_t
>> type.
>>
>> I have posted: https://patchwork.ozlabs.org/patch/1146830/
>>
>> I believe the above should fix the size mismatch.
>
> The patch appears to fix the problem. Thanks!
Thanks for testing it. If all your tests have passed please post your
Tested-by on that patch.
>
> [0] Steps to reproduce the issue reported in
> https://patchwork.ozlabs.org/patch/1090094/#2212555
Thanks for the instructions. I will make sure i test this
before posting any patch related to drivers/core/of
Best Regards,
Keerthy
>
> * Apply patch [1] to u-boot/master
> * u-boot (master) gcc --version
> gcc (Ubuntu 7.4.0-1ubuntu1~16.04~ppa1) 7.4.0
> * u-boot (master) make defconfig all -j12
> * u-boot (master) ./u-boot -d arch/sandbox/dts/test.dtb
>
> [1] Dirty patch to enable ASAN in sandbox U-Boot (v2019.10-rc2):
>
> diff --git a/Makefile b/Makefile
> index 3b0864ae8eaa..b8ef4f267ed5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -391,7 +391,7 @@ KBUILD_CPPFLAGS := -D__KERNEL__ -D__UBOOT__
>
> KBUILD_CFLAGS := -Wall -Wstrict-prototypes \
> -Wno-format-security \
> - -fno-builtin -ffreestanding $(CSTD_FLAG)
> + -fno-builtin -ffreestanding $(CSTD_FLAG) -fsanitize=address
> KBUILD_CFLAGS += -fshort-wchar -fno-strict-aliasing
> KBUILD_AFLAGS := -D__ASSEMBLY__
>
> @@ -768,6 +768,7 @@ PLATFORM_LIBGCC = arch/$(ARCH)/lib/lib.a
> else
> PLATFORM_LIBGCC := -L $(shell dirname `$(CC) $(c_flags) -print-libgcc-file-name`) -lgcc
> endif
> +PLATFORM_LIBS := -lasan $(PLATFORM_LIBS)
> PLATFORM_LIBS += $(PLATFORM_LIBGCC)
>
> ifdef CONFIG_CC_COVERAGE
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-08-14 10:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24 11:49 [U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index Keerthy
2019-04-24 23:59 ` Simon Glass
2019-05-22 13:21 ` Simon Glass
2019-07-09 19:38 ` Eugeniu Rosca
2019-08-13 9:33 ` Simon Glass
2019-08-13 9:55 ` Keerthy
2019-08-14 8:23 ` Keerthy
2019-08-14 9:47 ` Eugeniu Rosca
2019-08-14 10:18 ` Keerthy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.