[U-Boot] [PATCH 1/2] usb: composite: fix possible alignment issues

Heinrich Schuchardt xypron.glpk at gmx.de
Thu Nov 21 19:36:58 UTC 2019


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 <simon.k.r.goldschmidt at gmail.com>
> ---
>
>   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.

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;
>



More information about the U-Boot mailing list