Re: [syzbot] [usb?] UBSAN: array-index-out-of-bounds in usbhid_parse

[Nikita Zhandarovich]

Test to see that changes made to hid_descriptor are fine.

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
---
 drivers/hid/usbhid/hid-core.c       |  2 +-
 drivers/usb/gadget/function/f_fs.c  |  3 ++-
 drivers/usb/gadget/function/f_hid.c | 22 ++++++++++++++--------
 include/linux/hid.h                 |  2 +-
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index a6eb6fe6130d..eb4807785d6d 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1010,7 +1010,7 @@ static int usbhid_parse(struct hid_device *hid)
 		return -ENODEV;
 	}
 
-	if (hdesc->bLength < sizeof(struct hid_descriptor)) {
+	if (hdesc->bLength < struct_size(hdesc, desc, hdesc->bNumDescriptors)) {
 		dbg_hid("hid descriptor is too short\n");
 		return -EINVAL;
 	}
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 2dea9e42a0f8..a4b6d7cbf56d 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -2550,7 +2550,8 @@ static int __must_check ffs_do_single_desc(char *data, unsigned len,
 	case USB_TYPE_CLASS | 0x01:
 		if (*current_class == USB_INTERFACE_CLASS_HID) {
 			pr_vdebug("hid descriptor\n");
-			if (length != sizeof(struct hid_descriptor))
+			if (length < sizeof(struct hid_descriptor) +
+				     sizeof(struct hid_class_descriptor))
 				goto inv_length;
 			break;
 		} else if (*current_class == USB_INTERFACE_CLASS_CCID) {
diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 740311c4fa24..ec8c2e2d6812 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -139,13 +139,17 @@ static struct usb_interface_descriptor hidg_interface_desc = {
 };
 
 static struct hid_descriptor hidg_desc = {
-	.bLength			= sizeof hidg_desc,
+	.bLength			= struct_size(&hidg_desc, desc, 1),
 	.bDescriptorType		= HID_DT_HID,
 	.bcdHID				= cpu_to_le16(0x0101),
 	.bCountryCode			= 0x00,
 	.bNumDescriptors		= 0x1,
-	/*.desc[0].bDescriptorType	= DYNAMIC */
-	/*.desc[0].wDescriptorLenght	= DYNAMIC */
+	.desc				= {
+		{
+			.bDescriptorType	= 0, /* DYNAMIC */
+			.wDescriptorLength	= 0, /* DYNAMIC */
+		}
+	}
 };
 
 /* Super-Speed Support */
@@ -936,16 +940,18 @@ static int hidg_setup(struct usb_function *f,
 		switch (value >> 8) {
 		case HID_DT_HID:
 		{
-			struct hid_descriptor hidg_desc_copy = hidg_desc;
+			DEFINE_FLEX(struct hid_descriptor, hidg_desc_copy,
+				desc, bNumDescriptors, 1);
+			*hidg_desc_copy = hidg_desc;
 
 			VDBG(cdev, "USB_REQ_GET_DESCRIPTOR: HID\n");
-			hidg_desc_copy.desc[0].bDescriptorType = HID_DT_REPORT;
-			hidg_desc_copy.desc[0].wDescriptorLength =
+			hidg_desc_copy->desc[0].bDescriptorType = HID_DT_REPORT;
+			hidg_desc_copy->desc[0].wDescriptorLength =
 				cpu_to_le16(hidg->report_desc_length);
 
 			length = min_t(unsigned short, length,
-						   hidg_desc_copy.bLength);
-			memcpy(req->buf, &hidg_desc_copy, length);
+						   hidg_desc_copy->bLength);
+			memcpy(req->buf, hidg_desc_copy, length);
 			goto respond;
 			break;
 		}
diff --git a/include/linux/hid.h b/include/linux/hid.h
index cdc0dc13c87f..85a58ae2c4a0 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -739,7 +739,7 @@ struct hid_descriptor {
 	__u8  bCountryCode;
 	__u8  bNumDescriptors;
 
-	struct hid_class_descriptor desc[1];
+	struct hid_class_descriptor desc[] __counted_by(bNumDescriptors);
 } __attribute__ ((packed));
 
 #define HID_DEVICE(b, g, ven, prod)					\