| Message ID | 20250131151600.410242-1-n.zhandarovich@fintech.ru |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] HID: usbhid: fix recurrent out-of-bounds bug in usbhid_parse() | expand |
On Fri, Jan 31, 2025 at 06:15:58PM +0300, Nikita Zhandarovich wrote: > Syzbot reports [1] a reemerging out-of-bounds bug regarding hid > descriptors supposedly having unpredictable bNumDescriptors values in > usbhid_parse(). > > The issue stems from the fact that hid_class_descriptor is supposed > to be a flexible array, however it was sized as desc[1], using only > one element. Therefore, going beyond 1 element, courtesy of > bNumDescriptors, may lead to an error. > > Modify struct hid_descriptor by employing __counted_by macro, tying > together struct hid_class_descriptor desc[] and number of descriptors > bNumDescriptors. Also, fix places where this change affects work with > newly updated struct. > > [1] Syzbot report: > > UBSAN: array-index-out-of-bounds in drivers/hid/usbhid/hid-core.c:1024:7 > index 1 is out of range for type 'struct hid_class_descriptor[1]' > ... > Workqueue: usb_hub_wq hub_event > Call Trace: > <TASK> > __dump_stack lib/dump_stack.c:88 [inline] > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114 > ubsan_epilogue lib/ubsan.c:231 [inline] > __ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429 > usbhid_parse+0x5a7/0xc80 drivers/hid/usbhid/hid-core.c:1024 > hid_add_device+0x132/0x520 drivers/hid/hid-core.c:2790 > usbhid_probe+0xb38/0xea0 drivers/hid/usbhid/hid-core.c:1429 > usb_probe_interface+0x645/0xbb0 drivers/usb/core/driver.c:399 > really_probe+0x2b8/0xad0 drivers/base/dd.c:656 > __driver_probe_device+0x1a2/0x390 drivers/base/dd.c:798 > driver_probe_device+0x50/0x430 drivers/base/dd.c:828 > __device_attach_driver+0x2d6/0x530 drivers/base/dd.c:956 > bus_for_each_drv+0x24e/0x2e0 drivers/base/bus.c:457 > __device_attach+0x333/0x520 drivers/base/dd.c:1028 > bus_probe_device+0x189/0x260 drivers/base/bus.c:532 > device_add+0x8ff/0xca0 drivers/base/core.c:3720 > usb_set_configuration+0x1976/0x1fb0 drivers/usb/core/message.c:2210 > usb_generic_driver_probe+0x88/0x140 drivers/usb/core/generic.c:254 > usb_probe_device+0x1b8/0x380 drivers/usb/core/driver.c:294 > > 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: Nikita Zhandarovich <n.zhandarovich@fintech.ru> > --- > v1: https://lore.kernel.org/all/20240524120112.28076-1-n.zhandarovich@fintech.ru/ > > v2: Instead of essentially forcing usbhid_parse() to only check > the first descriptor, modify hid_descriptor struct to anticipate > multiple hid_class_descriptor in desc[] by utilizing __counted_by > macro. This change, in turn, requires several other ones wherever > that struct is used. Adjust commit description accordingly. > > P.S. Since syzbot currently breaks trying to reproduce the issue, > with or without this patch, I only managed to test it locally with > syz repros. Would greatly appreciate other people's effort to test > it as well. > > P.P.S. Terry Junge <linuxhid@cosmicgizmosystems.com> suggested a > different approach to this issue, see: > Link: https://lore.kernel.org/all/f7963a1d-e069-4ec9-82a1-e17fd324a44f@cosmicgizmosystems.com/ > > 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; > } I don't think you want this hunk in the patch. The existing logic will correctly adapt num_descriptors to a size that fits within hdesc->bLength. In theory, the above change could break a weird but working device that reported too high bNumDescriptors but still worked with what num_descriptors walks. > 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) { Same here, I think? This isn't needed unless I'm misunderstanding something about the fix. > 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) \ Otherwise, this looks correct to me.
On 1/31/25 23:12, Kees Cook wrote: > On Fri, Jan 31, 2025 at 06:15:58PM +0300, Nikita Zhandarovich wrote: >> Syzbot reports [1] a reemerging out-of-bounds bug regarding hid >> descriptors supposedly having unpredictable bNumDescriptors values in >> usbhid_parse(). >> >> The issue stems from the fact that hid_class_descriptor is supposed >> to be a flexible array, however it was sized as desc[1], using only >> one element. Therefore, going beyond 1 element, courtesy of >> bNumDescriptors, may lead to an error. >> >> Modify struct hid_descriptor by employing __counted_by macro, tying >> together struct hid_class_descriptor desc[] and number of descriptors >> bNumDescriptors. Also, fix places where this change affects work with >> newly updated struct. >> >> [1] Syzbot report: >> >> UBSAN: array-index-out-of-bounds in drivers/hid/usbhid/hid-core.c:1024:7 >> index 1 is out of range for type 'struct hid_class_descriptor[1]' >> ... >> Workqueue: usb_hub_wq hub_event >> Call Trace: >> <TASK> >> __dump_stack lib/dump_stack.c:88 [inline] >> dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114 >> ubsan_epilogue lib/ubsan.c:231 [inline] >> __ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429 >> usbhid_parse+0x5a7/0xc80 drivers/hid/usbhid/hid-core.c:1024 >> hid_add_device+0x132/0x520 drivers/hid/hid-core.c:2790 >> usbhid_probe+0xb38/0xea0 drivers/hid/usbhid/hid-core.c:1429 >> usb_probe_interface+0x645/0xbb0 drivers/usb/core/driver.c:399 >> really_probe+0x2b8/0xad0 drivers/base/dd.c:656 >> __driver_probe_device+0x1a2/0x390 drivers/base/dd.c:798 >> driver_probe_device+0x50/0x430 drivers/base/dd.c:828 >> __device_attach_driver+0x2d6/0x530 drivers/base/dd.c:956 >> bus_for_each_drv+0x24e/0x2e0 drivers/base/bus.c:457 >> __device_attach+0x333/0x520 drivers/base/dd.c:1028 >> bus_probe_device+0x189/0x260 drivers/base/bus.c:532 >> device_add+0x8ff/0xca0 drivers/base/core.c:3720 >> usb_set_configuration+0x1976/0x1fb0 drivers/usb/core/message.c:2210 >> usb_generic_driver_probe+0x88/0x140 drivers/usb/core/generic.c:254 >> usb_probe_device+0x1b8/0x380 drivers/usb/core/driver.c:294 >> >> 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: Nikita Zhandarovich <n.zhandarovich@fintech.ru> >> --- >> v1: https://lore.kernel.org/all/20240524120112.28076-1-n.zhandarovich@fintech.ru/ >> >> v2: Instead of essentially forcing usbhid_parse() to only check >> the first descriptor, modify hid_descriptor struct to anticipate >> multiple hid_class_descriptor in desc[] by utilizing __counted_by >> macro. This change, in turn, requires several other ones wherever >> that struct is used. Adjust commit description accordingly. >> >> P.S. Since syzbot currently breaks trying to reproduce the issue, >> with or without this patch, I only managed to test it locally with >> syz repros. Would greatly appreciate other people's effort to test >> it as well. >> >> P.P.S. Terry Junge <linuxhid@cosmicgizmosystems.com> suggested a >> different approach to this issue, see: >> Link: https://lore.kernel.org/all/f7963a1d-e069-4ec9-82a1-e17fd324a44f@cosmicgizmosystems.com/ >> >> 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; >> } > > I don't think you want this hunk in the patch. The existing logic will > correctly adapt num_descriptors to a size that fits within > hdesc->bLength. In theory, the above change could break a weird but > working device that reported too high bNumDescriptors but still worked > with what num_descriptors walks. > Thank you for mentioning this and you are right about possibly breaking a working device. However, sizeof(struct hid_descriptor) doesn't count flex array sizes. So, leaving this check as is will miss cases when hdesc->bLength >= sizeof(struct hid_descriptor) but short enough to have a desc[0] element. Maybe doing struct_size(hdesc, desc, 1) is better? Then we make sure that at least one mandatory hid_class_desriptor is there. >> 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) { > > Same here, I think? This isn't needed unless I'm misunderstanding > something about the fix. Once again, sizeof(struct hid_descriptor) will not count struct hid_class_descriptor desc[] __counted_by(...) so even correct and predictable lengths will fail the check. We need to test length against hid_descriptor with at least one element of its flex array. I would prefer struct_size() here as well but it's not optimal in this case. > >> 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) \ > > Otherwise, this looks correct to me. > Regards, Nikita
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) \
Syzbot reports [1] a reemerging out-of-bounds bug regarding hid descriptors supposedly having unpredictable bNumDescriptors values in usbhid_parse(). The issue stems from the fact that hid_class_descriptor is supposed to be a flexible array, however it was sized as desc[1], using only one element. Therefore, going beyond 1 element, courtesy of bNumDescriptors, may lead to an error. Modify struct hid_descriptor by employing __counted_by macro, tying together struct hid_class_descriptor desc[] and number of descriptors bNumDescriptors. Also, fix places where this change affects work with newly updated struct. [1] Syzbot report: UBSAN: array-index-out-of-bounds in drivers/hid/usbhid/hid-core.c:1024:7 index 1 is out of range for type 'struct hid_class_descriptor[1]' ... Workqueue: usb_hub_wq hub_event Call Trace: <TASK> __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114 ubsan_epilogue lib/ubsan.c:231 [inline] __ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429 usbhid_parse+0x5a7/0xc80 drivers/hid/usbhid/hid-core.c:1024 hid_add_device+0x132/0x520 drivers/hid/hid-core.c:2790 usbhid_probe+0xb38/0xea0 drivers/hid/usbhid/hid-core.c:1429 usb_probe_interface+0x645/0xbb0 drivers/usb/core/driver.c:399 really_probe+0x2b8/0xad0 drivers/base/dd.c:656 __driver_probe_device+0x1a2/0x390 drivers/base/dd.c:798 driver_probe_device+0x50/0x430 drivers/base/dd.c:828 __device_attach_driver+0x2d6/0x530 drivers/base/dd.c:956 bus_for_each_drv+0x24e/0x2e0 drivers/base/bus.c:457 __device_attach+0x333/0x520 drivers/base/dd.c:1028 bus_probe_device+0x189/0x260 drivers/base/bus.c:532 device_add+0x8ff/0xca0 drivers/base/core.c:3720 usb_set_configuration+0x1976/0x1fb0 drivers/usb/core/message.c:2210 usb_generic_driver_probe+0x88/0x140 drivers/usb/core/generic.c:254 usb_probe_device+0x1b8/0x380 drivers/usb/core/driver.c:294 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: Nikita Zhandarovich <n.zhandarovich@fintech.ru> --- v1: https://lore.kernel.org/all/20240524120112.28076-1-n.zhandarovich@fintech.ru/ v2: Instead of essentially forcing usbhid_parse() to only check the first descriptor, modify hid_descriptor struct to anticipate multiple hid_class_descriptor in desc[] by utilizing __counted_by macro. This change, in turn, requires several other ones wherever that struct is used. Adjust commit description accordingly. P.S. Since syzbot currently breaks trying to reproduce the issue, with or without this patch, I only managed to test it locally with syz repros. Would greatly appreciate other people's effort to test it as well. P.P.S. Terry Junge <linuxhid@cosmicgizmosystems.com> suggested a different approach to this issue, see: Link: https://lore.kernel.org/all/f7963a1d-e069-4ec9-82a1-e17fd324a44f@cosmicgizmosystems.com/ 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(-)