On Mon, Jan 20, 2020 at 01:15:41PM +0000, David Howells wrote:
> Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> > 2759 struct {
> > 2760 struct usb_notification n;
> > 2761 char more_name[USB_NOTIFICATION_MAX_NAME_LEN -
> > 2762 (sizeof(struct usb_notification) -
> > 2763 offsetof(struct usb_notification, name))];
> > 2764 } n;
> > 2765
> > 2766 name_len = strlen(devname);
> > 2767 name_len = min_t(size_t, name_len, USB_NOTIFICATION_MAX_NAME_LEN);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > This limit is too high. It should be USB_NOTIFICATION_MAX_NAME_LEN -
> > sizeof(struct usb_notification). or just
> > "min_t(size_t, name_len, sizeof(n.more_name));". n.n.name[] is a
> > zero size array.
>
> No. It's not that simple. If you look at the struct:
>
> struct usb_notification {
> struct watch_notification watch;
> __u32 error;
> __u32 reserved;
> __u8 name_len;
> __u8 name[0];
> };
>
> There are at least 3, if not 7, bytes of padding after name[] as the struct is
> not packed - and isn't necessarily rounded up to a multiple of 8 bytes either.
> If you look at the definition of more_name[] above, you'll see:
>
> USB_NOTIFICATION_MAX_NAME_LEN -
> (sizeof(struct usb_notification) -
> offsetof(struct usb_notification, name))
>
> That calculates the amount of padding and then subtracts it from the amount of
> name bufferage required.
>
> USB_NOTIFICATION_MAX_NAME_LEN is 63, which is 64 minus one for the length.
Ah yes... You're right. I didn't think about padding. And even if I
had, I would have thought the hole would have gone before name[] but
it comes after as you say.
regards,
dan carpenter
