[U-Boot] [RESEND PATCH] core: ofnode: Add ofnode_get_addr_size_index

Wed Aug 14 08:23:50 UTC 2019

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 at 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 at ti.com>
>>>> Reviewed-by: Simon Glass <sjg at 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 at 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