HID: usbhid: Eliminate recurrent out-of-bounds bug in usbhid_parse()

commit fe7f7ac8e0c708446ff017453add769ffc15deed upstream.

Update struct hid_descriptor to better reflect the mandatory and
optional parts of the HID Descriptor as per USB HID 1.11 specification.
Note: the kernel currently does not parse any optional HID class
descriptors, only the mandatory report descriptor.

Update all references to member element desc[0] to rpt_desc.

Add test to verify bLength and bNumDescriptors values are valid.

Replace the for loop with direct access to the mandatory HID class
descriptor member for the report descriptor. This eliminates the
possibility of getting an out-of-bounds fault.

Add a warning message if the HID descriptor contains any unsupported
optional HID class descriptors.

Reported-by: syzbot+c52569baf0c843f35495@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c52569baf0c843f35495
Fixes: f043bfc98c19 ("HID: usbhid: fix out-of-bounds bug")
Cc: stable@vger.kernel.org
Signed-off-by: Terry Junge <linuxhid@cosmicgizmosystems.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Terry Junge <linuxhid@cosmicgizmosystems.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index b7704dd6809dc8786ed5e7db61cbaa0b9847ef07..bf77cfb723d5d630d4fcc20f9985b0a494cbbe11 100644 (file)
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -199,7 +199,8 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
        if (!input_device->hid_desc)
                goto cleanup;
 
-       input_device->report_desc_size = desc->desc[0].wDescriptorLength;
+       input_device->report_desc_size = le16_to_cpu(
+                                       desc->rpt_desc.wDescriptorLength);
        if (input_device->report_desc_size == 0) {
                input_device->dev_info_status = -EINVAL;
                goto cleanup;
@@ -217,7 +218,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
 
        memcpy(input_device->report_desc,
               ((unsigned char *)desc) + desc->bLength,
-              desc->desc[0].wDescriptorLength);
+              le16_to_cpu(desc->rpt_desc.wDescriptorLength));
 
        /* Send the ack */
        memset(&ack, 0, sizeof(struct mousevsc_prt_msg));

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 009a0469d54f6aa4ebe11b5f6f7af8146eeb620f..c3b104c72e496758924c344a690842e10705da53 100644 (file)
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -984,12 +984,11 @@ static int usbhid_parse(struct hid_device *hid)
        struct usb_host_interface *interface = intf->cur_altsetting;
        struct usb_device *dev = interface_to_usbdev (intf);
        struct hid_descriptor *hdesc;
+       struct hid_class_descriptor *hcdesc;
        u32 quirks = 0;
        unsigned int rsize = 0;
        char *rdesc;
-       int ret, n;
-       int num_descriptors;
-       size_t offset = offsetof(struct hid_descriptor, desc);
+       int ret;
 
        quirks = hid_lookup_quirk(hid);
 
@@ -1011,20 +1010,19 @@ static int usbhid_parse(struct hid_device *hid)
                return -ENODEV;
        }
 
-       if (hdesc->bLength < sizeof(struct hid_descriptor)) {
-               dbg_hid("hid descriptor is too short\n");
+       if (!hdesc->bNumDescriptors ||
+           hdesc->bLength != sizeof(*hdesc) +
+                             (hdesc->bNumDescriptors - 1) * sizeof(*hcdesc)) {
+               dbg_hid("hid descriptor invalid, bLen=%hhu bNum=%hhu\n",
+                       hdesc->bLength, hdesc->bNumDescriptors);
                return -EINVAL;
        }
 
        hid->version = le16_to_cpu(hdesc->bcdHID);
        hid->country = hdesc->bCountryCode;
 
-       num_descriptors = min_t(int, hdesc->bNumDescriptors,
-              (hdesc->bLength - offset) / sizeof(struct hid_class_descriptor));
-
-       for (n = 0; n < num_descriptors; n++)
-               if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT)
-                       rsize = le16_to_cpu(hdesc->desc[n].wDescriptorLength);
+       if (hdesc->rpt_desc.bDescriptorType == HID_DT_REPORT)
+               rsize = le16_to_cpu(hdesc->rpt_desc.wDescriptorLength);
 
        if (!rsize || rsize > HID_MAX_DESCRIPTOR_SIZE) {
                dbg_hid("weird size of report descriptor (%u)\n", rsize);
@@ -1052,6 +1050,11 @@ static int usbhid_parse(struct hid_device *hid)
                goto err;
        }
 
+       if (hdesc->bNumDescriptors > 1)
+               hid_warn(intf,
+                       "%u unsupported optional hid class descriptors\n",
+                       (int)(hdesc->bNumDescriptors - 1));
+
        hid->quirks |= quirks;
 
        return 0;

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index ba018aeb21d8cab64c3a0817ae362458cb8ed772..2f30699f0426fb0a7e988666d549a7f822463cfb 100644 (file)
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -114,8 +114,8 @@ static struct hid_descriptor hidg_desc = {
        .bcdHID                         = cpu_to_le16(0x0101),
        .bCountryCode                   = 0x00,
        .bNumDescriptors                = 0x1,
-       /*.desc[0].bDescriptorType      = DYNAMIC */
-       /*.desc[0].wDescriptorLenght    = DYNAMIC */
+       /*.rpt_desc.bDescriptorType     = DYNAMIC */
+       /*.rpt_desc.wDescriptorLength   = DYNAMIC */
 };
 
 /* Super-Speed Support */
@@ -724,8 +724,8 @@ static int hidg_setup(struct usb_function *f,
                        struct hid_descriptor 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.rpt_desc.bDescriptorType = HID_DT_REPORT;
+                       hidg_desc_copy.rpt_desc.wDescriptorLength =
                                cpu_to_le16(hidg->report_desc_length);
 
                        length = min_t(unsigned short, length,
@@ -966,8 +966,8 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
         * We can use hidg_desc struct here but we should not relay
         * that its content won't change after returning from this function.
         */
-       hidg_desc.desc[0].bDescriptorType = HID_DT_REPORT;
-       hidg_desc.desc[0].wDescriptorLength =
+       hidg_desc.rpt_desc.bDescriptorType = HID_DT_REPORT;
+       hidg_desc.rpt_desc.wDescriptorLength =
                cpu_to_le16(hidg->report_desc_length);
 
        hidg_hs_in_ep_desc.bEndpointAddress =

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 9e306bf9959df65b9a7504d663d5f0c186e7b9a0..03627c96d81457392129433e4f6bd19d4bd1bd39 100644 (file)
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -674,8 +674,9 @@ struct hid_descriptor {
        __le16 bcdHID;
        __u8  bCountryCode;
        __u8  bNumDescriptors;
+       struct hid_class_descriptor rpt_desc;
 
-       struct hid_class_descriptor desc[1];
+       struct hid_class_descriptor opt_descs[];
 } __attribute__ ((packed));
 
 #define HID_DEVICE(b, g, ven, prod)                                    \