[U-Boot] [PATCH] usb: dwc3: fixes crash in dwc3 driver due to types size mismatch

[Marek Vasut]

On 07/22/2016 07:10 AM, Masahiro Yamada wrote:
> Hi.
> 
> 
> 2016-07-21 21:29 GMT+09:00 B, Ravi <ravibabu at ti.com>:
>> Hi Marek
>>
>>>> The crash at dwc3 driver observed due to offset misalignment of
>>>> structure members across files causing wrong code generation and leads
>>>> to crash, the issue is found during dfu test.
>>>>
>>>> For instance, ther is is mismatch in code generation to access the
>>>> address of structure member dwc->dep[0] in gadget.c and ep0.c. This
>>>> leads to NULL pointer reference casuing the crash. The inclusion of
>>>> common.h fixes the issue.
>>
>>> Please explain why this patch fixes the issue.
>>
>> Ok I will explain, due to the commit[1] the resource_size_t size has increased to 8 bytes (64 bit), compared to earlier 32 bit (4bytes) and the definition is moved to includes/linux/types.h from asm.h. Due to this change the code generated in gadget.c is correct, due to inclusion of right header file (common.h, which includes linux/types.h). Whereas, the ep0.c does not includes common.h, hence  size of resources_size_t is 4 bytes, causing wrong offset code generated for structure members which includes resource_size_t, which leads to pointing to wrong offset location causing the crash.
> 
> 
> I will explain it more precisely.
> 
> 
> Both gadget.c and ep.c include <linux/types.h>
> 
> See,
>  include/linux/types.h is included
>    from include/linux/kernel.h  included
>    from drivers/usb/dwc3/ep0.c
> 
> 
> So, <linux/types.h> is not a problem.
> 
> 
> The root cause of problem is:
> gadget.c include <config.h>, but ep0.c does not.
> 
> 
> If <config.h> is not included, any CONFIGs
> from the board header are defined.
> 
> 
> The size of phys_addr_t depends on CONFIG_PHYS_64BIT
> as you see in:
> 
> 
> #ifdef CONFIG_PHYS_64BIT
> typedef unsigned long long phys_addr_t;
> typedef unsigned long long phys_size_t;
> #else
> /* DMA addresses are 32-bits wide */
> typedef unsigned long phys_addr_t;
> typedef unsigned long phys_size_t;
> #endif
> 
> 
> So, phys_addr_t is 8 byte in gadget.c
> and phys_addr_t is 4 byte in ep0.c
> 
> 
> My commit changed resource_size_t
> based on phys_addr_t, so it triggered
> the problem for DWC3, which had already potentially existed.
> 
> 
> CONFIGs in Kconfig are guaranteed to be defined for all files,
> but CONFIGs in board headers are not.
> 
> So we need to make sure to add
> #include <common.h> (or #include <config.h>)
> in each source file.
> 
> 
> So, your patch is doing a right thing.
> 
> I will issue my Reviewed-by when you update the git-log.
> 
> 
> (Moving CONFIG_PHYS_64BIT is a right thing as well)
> 
> 
Thanks for the sensible and understandable explanation.

-- 
Best regards,
Marek Vasut