From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Goldschmidt Date: Thu, 21 Nov 2019 22:12:41 +0100 Subject: [U-Boot] [PATCH 1/2] usb: composite: fix possible alignment issues In-Reply-To: References: <20191112212643.915-1-simon.k.r.goldschmidt@gmail.com> Message-ID: <89b1b3c4-6fe7-7c26-9da1-c968deb5f997@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Am 21.11.2019 um 20:36 schrieb Heinrich Schuchardt: > On 11/12/19 10:26 PM, Simon Goldschmidt wrote: >> Since upgrading to gcc9, warnings are issued: >> "taking address of packed member of ‘...’ may result in an unaligned >> pointer value" >> >> Fix this by converting two functions to use unaligned access since packed >> structures may be on an unaligned address, depending on USB hardware. >> >> Signed-off-by: Simon Goldschmidt >> --- >> >> drivers/usb/gadget/composite.c | 24 +++++++++++++++++------- >> 1 file changed, 17 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c >> index 618a7d5016..cfc9512caa 100644 >> --- a/drivers/usb/gadget/composite.c >> +++ b/drivers/usb/gadget/composite.c >> @@ -12,8 +12,16 @@ >> >> #define USB_BUFSIZ 4096 >> >> +/* Helper type for accessing packed u16 pointers */ >> +typedef struct { __le16 val; } __packed __le16_packed; >> + >> static struct usb_composite_driver *composite; >> >> +static inline void le16_add_cpu_packed(__le16_packed *var, u16 val) >> +{ >> + var->val = cpu_to_le16(le16_to_cpu(var->val) + val); >> +} >> + >> /** >> * usb_add_function() - add a function to a configuration >> * @config: the configuration >> @@ -480,20 +488,20 @@ done: >> * the host side. >> */ >> >> -static void collect_langs(struct usb_gadget_strings **sp, __le16 *buf) >> +static void collect_langs(struct usb_gadget_strings **sp, void *buf) > > With this patch and GCC 9.2.1 I get: > > In file included from drivers/usb/gadget/g_dnl.c:24: > drivers/usb/gadget/composite.c: In function ‘collect_langs’: > drivers/usb/gadget/composite.c:500:41: error: dereferencing ‘void *’ > pointer [-Werror] > 500 | for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) { > | ^ > drivers/usb/gadget/composite.c:500:35: error: comparison of distinct > pointer types lacks a cast [-Werror] > 500 | for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) { > > void does not have defined size and so cannot be subscripted. Right, thanks for testing. I hate it that we don't have -Werror enabled, as this just slipped through with all the output on the screen. I'll send v2 fixing that. Regards, Simon > > Best regards > > Heinrich > >> { >> const struct usb_gadget_strings *s; >> u16 language; >> - __le16 *tmp; >> + __le16_packed *tmp; >> >> while (*sp) { >> s = *sp; >> language = cpu_to_le16(s->language); >> - for (tmp = buf; *tmp && tmp < &buf[126]; tmp++) { >> - if (*tmp == language) >> + for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) { >> + if (tmp->val == language) >> goto repeat; >> } >> - *tmp++ = language; >> + tmp->val = language; >> repeat: >> sp++; >> } >> @@ -705,7 +713,8 @@ static int bos_desc(struct usb_composite_dev *cdev) >> */ >> usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength); >> bos->bNumDeviceCaps++; >> - le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE); >> + le16_add_cpu_packed((__le16_packed *)&bos->wTotalLength, >> + USB_DT_USB_EXT_CAP_SIZE); >> usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE; >> usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY; >> usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT; >> @@ -721,7 +730,8 @@ static int bos_desc(struct usb_composite_dev *cdev) >> >> ss_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength); >> bos->bNumDeviceCaps++; >> - le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SS_CAP_SIZE); >> + le16_add_cpu_packed((__le16_packed *)&bos->wTotalLength, >> + USB_DT_USB_SS_CAP_SIZE); >> ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE; >> ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY; >> ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE; >> >