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

[Masahiro Yamada]

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)


-- 
Best Regards
Masahiro Yamada