From: Terry Junge <[email protected]> Sent: Wednesday, March 12, 2025 3:24 PM > > 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: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=c52569baf0c843f35495 > Fixes: f043bfc98c19 ("HID: usbhid: fix out-of-bounds bug") > Cc: [email protected] > Signed-off-by: Terry Junge <[email protected]> > --- > v1: Remove unnecessary for loop searching for the report descriptor size. > v2: Fix compiler warning. > base-commit: 58c9bf3363e596d744f56616d407278ef5f97f5a > > P.S. This is an alternative to the solution proposed by Nikita Zhandarovich > <[email protected]> > Link: > https://lore.kernel.org/all/[email protected]/ > > include/linux/hid.h | 3 ++- > drivers/usb/gadget/function/f_hid.c | 12 ++++++------ > drivers/hid/hid-hyperv.c | 4 ++-- > drivers/hid/usbhid/hid-core.c | 25 ++++++++++++++----------- > 4 files changed, 24 insertions(+), 20 deletions(-) > > diff --git a/include/linux/hid.h b/include/linux/hid.h > index cdc0dc13c87f..7abc8c74bdd5 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -738,8 +738,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) \ > diff --git a/drivers/usb/gadget/function/f_hid.c > b/drivers/usb/gadget/function/f_hid.c > index 740311c4fa24..c7a05f842745 100644 > --- a/drivers/usb/gadget/function/f_hid.c > +++ b/drivers/usb/gadget/function/f_hid.c > @@ -144,8 +144,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 */ > @@ -939,8 +939,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, > @@ -1210,8 +1210,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/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 a6eb6fe6130d..f8b853180680 100644 > --- a/drivers/hid/usbhid/hid-core.c > +++ b/drivers/hid/usbhid/hid-core.c > @@ -983,12 +983,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); > > @@ -1010,20 +1009,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); > @@ -1051,6 +1049,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; > -- > 2.43.0 >
For the hid-hyperv.c changes, Reviewed-by: Michael Kelley <[email protected]>
