Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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 <linuxhid@cosmicgizmosystems.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>

authored by

Terry Junge and committed by
Jiri Kosina
fe7f7ac8 5e06802b

+24 -20
+2 -2
drivers/hid/hid-hyperv.c
··· 192 192 goto cleanup; 193 193 194 194 input_device->report_desc_size = le16_to_cpu( 195 - desc->desc[0].wDescriptorLength); 195 + desc->rpt_desc.wDescriptorLength); 196 196 if (input_device->report_desc_size == 0) { 197 197 input_device->dev_info_status = -EINVAL; 198 198 goto cleanup; ··· 210 210 211 211 memcpy(input_device->report_desc, 212 212 ((unsigned char *)desc) + desc->bLength, 213 - le16_to_cpu(desc->desc[0].wDescriptorLength)); 213 + le16_to_cpu(desc->rpt_desc.wDescriptorLength)); 214 214 215 215 /* Send the ack */ 216 216 memset(&ack, 0, sizeof(struct mousevsc_prt_msg));
+14 -11
drivers/hid/usbhid/hid-core.c
··· 984 984 struct usb_host_interface *interface = intf->cur_altsetting; 985 985 struct usb_device *dev = interface_to_usbdev (intf); 986 986 struct hid_descriptor *hdesc; 987 + struct hid_class_descriptor *hcdesc; 987 988 u32 quirks = 0; 988 989 unsigned int rsize = 0; 989 990 char *rdesc; 990 - int ret, n; 991 - int num_descriptors; 992 - size_t offset = offsetof(struct hid_descriptor, desc); 991 + int ret; 993 992 994 993 quirks = hid_lookup_quirk(hid); 995 994 ··· 1010 1011 return -ENODEV; 1011 1012 } 1012 1013 1013 - if (hdesc->bLength < sizeof(struct hid_descriptor)) { 1014 - dbg_hid("hid descriptor is too short\n"); 1014 + if (!hdesc->bNumDescriptors || 1015 + hdesc->bLength != sizeof(*hdesc) + 1016 + (hdesc->bNumDescriptors - 1) * sizeof(*hcdesc)) { 1017 + dbg_hid("hid descriptor invalid, bLen=%hhu bNum=%hhu\n", 1018 + hdesc->bLength, hdesc->bNumDescriptors); 1015 1019 return -EINVAL; 1016 1020 } 1017 1021 1018 1022 hid->version = le16_to_cpu(hdesc->bcdHID); 1019 1023 hid->country = hdesc->bCountryCode; 1020 1024 1021 - num_descriptors = min_t(int, hdesc->bNumDescriptors, 1022 - (hdesc->bLength - offset) / sizeof(struct hid_class_descriptor)); 1023 - 1024 - for (n = 0; n < num_descriptors; n++) 1025 - if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT) 1026 - rsize = le16_to_cpu(hdesc->desc[n].wDescriptorLength); 1025 + if (hdesc->rpt_desc.bDescriptorType == HID_DT_REPORT) 1026 + rsize = le16_to_cpu(hdesc->rpt_desc.wDescriptorLength); 1027 1027 1028 1028 if (!rsize || rsize > HID_MAX_DESCRIPTOR_SIZE) { 1029 1029 dbg_hid("weird size of report descriptor (%u)\n", rsize); ··· 1049 1051 dbg_hid("parsing report descriptor failed\n"); 1050 1052 goto err; 1051 1053 } 1054 + 1055 + if (hdesc->bNumDescriptors > 1) 1056 + hid_warn(intf, 1057 + "%u unsupported optional hid class descriptors\n", 1058 + (int)(hdesc->bNumDescriptors - 1)); 1052 1059 1053 1060 hid->quirks |= quirks; 1054 1061
+6 -6
drivers/usb/gadget/function/f_hid.c
··· 144 144 .bcdHID = cpu_to_le16(0x0101), 145 145 .bCountryCode = 0x00, 146 146 .bNumDescriptors = 0x1, 147 - /*.desc[0].bDescriptorType = DYNAMIC */ 148 - /*.desc[0].wDescriptorLenght = DYNAMIC */ 147 + /*.rpt_desc.bDescriptorType = DYNAMIC */ 148 + /*.rpt_desc.wDescriptorLength = DYNAMIC */ 149 149 }; 150 150 151 151 /* Super-Speed Support */ ··· 939 939 struct hid_descriptor hidg_desc_copy = hidg_desc; 940 940 941 941 VDBG(cdev, "USB_REQ_GET_DESCRIPTOR: HID\n"); 942 - hidg_desc_copy.desc[0].bDescriptorType = HID_DT_REPORT; 943 - hidg_desc_copy.desc[0].wDescriptorLength = 942 + hidg_desc_copy.rpt_desc.bDescriptorType = HID_DT_REPORT; 943 + hidg_desc_copy.rpt_desc.wDescriptorLength = 944 944 cpu_to_le16(hidg->report_desc_length); 945 945 946 946 length = min_t(unsigned short, length, ··· 1210 1210 * We can use hidg_desc struct here but we should not relay 1211 1211 * that its content won't change after returning from this function. 1212 1212 */ 1213 - hidg_desc.desc[0].bDescriptorType = HID_DT_REPORT; 1214 - hidg_desc.desc[0].wDescriptorLength = 1213 + hidg_desc.rpt_desc.bDescriptorType = HID_DT_REPORT; 1214 + hidg_desc.rpt_desc.wDescriptorLength = 1215 1215 cpu_to_le16(hidg->report_desc_length); 1216 1216 1217 1217 hidg_hs_in_ep_desc.bEndpointAddress =
+2 -1
include/linux/hid.h
··· 740 740 __le16 bcdHID; 741 741 __u8 bCountryCode; 742 742 __u8 bNumDescriptors; 743 + struct hid_class_descriptor rpt_desc; 743 744 744 - struct hid_class_descriptor desc[1]; 745 + struct hid_class_descriptor opt_descs[]; 745 746 } __attribute__ ((packed)); 746 747 747 748 #define HID_DEVICE(b, g, ven, prod) \