From fe7f7ac8e0c708446ff017453add769ffc15deed Mon Sep 17 00:00:00 2001 From: Terry Junge Date: Wed, 12 Mar 2025 15:23:31 -0700 Subject: HID: usbhid: Eliminate recurrent out-of-bounds bug in usbhid_parse() 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 Reviewed-by: Michael Kelley Signed-off-by: Jiri Kosina --- drivers/hid/hid-hyperv.c | 4 ++-- drivers/hid/usbhid/hid-core.c | 25 ++++++++++++++----------- 2 files changed, 16 insertions(+), 13 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c index 0fb210e40a41..9eafff0b6ea4 100644 --- a/drivers/hid/hid-hyperv.c +++ b/drivers/hid/hid-hyperv.c @@ -192,7 +192,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device, goto cleanup; input_device->report_desc_size = le16_to_cpu( - desc->desc[0].wDescriptorLength); + desc->rpt_desc.wDescriptorLength); if (input_device->report_desc_size == 0) { input_device->dev_info_status = -EINVAL; goto cleanup; @@ -210,7 +210,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device, memcpy(input_device->report_desc, ((unsigned char *)desc) + desc->bLength, - le16_to_cpu(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 44c2351b870f..fc2b92dfc242 100644 --- 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; -- cgit v1.2.3-59-g8ed1b From a058002358b7aaf14674b2a0daa5194bfa5720a1 Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Thu, 10 Apr 2025 12:59:27 +0530 Subject: HID: quirks: Add HID_QUIRK_IGNORE_MOUSE quirk Some USB HID mice have drivers both in HID as well as a separate USB driver. The already existing hid_mouse_ignore_list in hid-quirks manages this, but is not yet configurable by usbhid.quirks, unlike all others like hid_ignore_list. Thus in some HID devices, where the vendor provides USB drivers only for the mouse and lets keyboard handled by the generic hid drivers, presence of such a quirk prevents the user from compiling hid core again to add the device to the table. Signed-off-by: Aditya Garg Signed-off-by: Jiri Kosina --- drivers/hid/hid-quirks.c | 5 ++++- include/linux/hid.h | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/hid') diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index 646171598e41..e36cbe78d67e 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -1061,7 +1061,7 @@ bool hid_ignore(struct hid_device *hdev) } if (hdev->type == HID_TYPE_USBMOUSE && - hid_match_id(hdev, hid_mouse_ignore_list)) + hdev->quirks & HID_QUIRK_IGNORE_MOUSE) return true; return !!hid_match_id(hdev, hid_ignore_list); @@ -1265,6 +1265,9 @@ static unsigned long hid_gets_squirk(const struct hid_device *hdev) if (hid_match_id(hdev, hid_ignore_list)) quirks |= HID_QUIRK_IGNORE; + if (hid_match_id(hdev, hid_mouse_ignore_list)) + quirks |= HID_QUIRK_IGNORE_MOUSE; + if (hid_match_id(hdev, hid_have_special_driver)) quirks |= HID_QUIRK_HAVE_SPECIAL_DRIVER; diff --git a/include/linux/hid.h b/include/linux/hid.h index daae1d6d11a7..a1305210b2fd 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -357,6 +357,7 @@ struct hid_item { * | @HID_QUIRK_INPUT_PER_APP: * | @HID_QUIRK_X_INVERT: * | @HID_QUIRK_Y_INVERT: + * | @HID_QUIRK_IGNORE_MOUSE: * | @HID_QUIRK_SKIP_OUTPUT_REPORTS: * | @HID_QUIRK_SKIP_OUTPUT_REPORT_ID: * | @HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP: @@ -382,6 +383,7 @@ struct hid_item { #define HID_QUIRK_INPUT_PER_APP BIT(11) #define HID_QUIRK_X_INVERT BIT(12) #define HID_QUIRK_Y_INVERT BIT(13) +#define HID_QUIRK_IGNORE_MOUSE BIT(14) #define HID_QUIRK_SKIP_OUTPUT_REPORTS BIT(16) #define HID_QUIRK_SKIP_OUTPUT_REPORT_ID BIT(17) #define HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP BIT(18) -- cgit v1.2.3-59-g8ed1b