[systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

[Lennart Poettering]

On Tue, 10.03.15 19:13, Shawn Landden (shawn at churchofgit.com) wrote:

> @@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device *dev, char *ifs_str, size_t len
>          int pos = 0;
>          unsigned strpos = 0;
>          struct usb_interface_descriptor {
> -                u_int8_t        bLength;
> -                u_int8_t        bDescriptorType;
> -                u_int8_t        bInterfaceNumber;
> -                u_int8_t        bAlternateSetting;
> -                u_int8_t        bNumEndpoints;
> -                u_int8_t        bInterfaceClass;
> -                u_int8_t        bInterfaceSubClass;
> -                u_int8_t        bInterfaceProtocol;
> -                u_int8_t        iInterface;
> +                uint8_t        bLength;
> +                uint8_t        bDescriptorType;
> +                uint8_t        bInterfaceNumber;
> +                uint8_t        bAlternateSetting;
> +                uint8_t        bNumEndpoints;
> +                uint8_t        bInterfaceClass;
> +                uint8_t        bInterfaceSubClass;
> +                uint8_t        bInterfaceProtocol;
> +                uint8_t        iInterface;
>          } __attribute__((packed));

This part has already been fixed with a different patch.

>  
>          if (asprintf(&filename, "%s/descriptors", udev_device_get_syspath(dev)) < 0)
> @@ -179,21 +180,20 @@ static int dev_if_packed_info(struct udev_device *dev, char *ifs_str, size_t len
>  
>          ifs_str[0] = '\0';
>          while (pos < size && strpos+7 < len-2) {
> -                struct usb_interface_descriptor *desc;
> +                char *desc = &buf[pos];
>                  char if_str[8];
>  
> -                desc = (struct usb_interface_descriptor *) &buf[pos];
> -                if (desc->bLength < 3)
> +                if (desc[offsetof(struct usb_interface_descriptor, bLength)] < 3)
>                          break;
> -                pos += desc->bLength;
> +                pos += desc[offsetof(struct usb_interface_descriptor, bLength)];
>  
> -                if (desc->bDescriptorType != USB_DT_INTERFACE)
> +                if (desc[offsetof(struct usb_interface_descriptor, bDescriptorType)] != USB_DT_INTERFACE)
>                          continue;
>  
>                  if (snprintf(if_str, 8, ":%02x%02x%02x",
> -                             desc->bInterfaceClass,
> -                             desc->bInterfaceSubClass,
> -                             desc->bInterfaceProtocol) != 7)
> +                             desc[offsetof(struct usb_interface_descriptor, bInterfaceClass)],
> +                             desc[offsetof(struct usb_interface_descriptor, bInterfaceSubClass)],
> +                             desc[offsetof(struct usb_interface_descriptor, bInterfaceProtocol)]) != 7)
>                          continue;

This call doesn't look pretty. I don#t really care too much about the
strict aliasing issues, but if you really want to fix this, then this
should probably be done with a union, like the other cases.

Lennart

-- 
Lennart Poettering, Red Hat