From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Fri, 22 Jul 2016 07:55:11 -0400 Subject: [U-Boot] [PATCH] usb: dwc3: fixes crash in dwc3 driver due to types size mismatch In-Reply-To: References: <1469097666-2758-1-git-send-email-ravibabu@ti.com> <23f5352b-213a-05fa-66b0-4c3d78073824@denx.de> <6C6B28D4DC342643927BEAFCE8707BF67629D4A6@DBDE04.ent.ti.com> Message-ID: <20160722115511.GL1623@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Fri, Jul 22, 2016 at 02:10:22PM +0900, Masahiro Yamada wrote: > Hi. > > > 2016-07-21 21:29 GMT+09:00 B, Ravi : > > 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 > > See, > include/linux/types.h is included > from include/linux/kernel.h included > from drivers/usb/dwc3/ep0.c > > > So, is not a problem. > > > The root cause of problem is: > gadget.c include , but ep0.c does not. > > > If 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 (or #include ) > 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) Can we move PHYS_64BIT to Kconfig instead here please? This is the kind of thing we should be able to select based on SoC / board. Thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: