Re: linux-next boot error: KASAN: slab-out-of-bounds Read in post_usb_notification

[David Howells]

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.

>   2771          memcpy(n.n.name, devname, n_len);
>                                           ^^^^^
> name_len was intended here.

Yeah.  I think that's actually the bug.  n_len is the length of the entire
notification record.

David