Em Tue, 9 Apr 2024 07:36:34 -0700
Nikita Zhandarovich <n.zhandarovich@xxxxxxxxxx> escreveu:
> Syzkaller reported a warning [1] in smsusb_submit_urb() which occurs
> if an attempt is made to send a bulk URB using the wrong endpoint
> type. The current approach to perform endpoint checking does not
> explicitly check if an endpoint in question has its type set to bulk.
>
> Fix this issue by using functions usb_endpoint_is_bulk_XXX() to
> enable testing for correct ep types.
>
> This patch has not been tested on real hardware.
>
> [1] Syzkaller report:
> usb 1-1: string descriptor 0 read error: -71
> smsusb:smsusb_probe: board id=2, interface number 0
> smsusb:siano_media_device_register: media controller created
> ------------[ cut here ]------------
> usb 1-1: BOGUS urb xfer, pipe 3 != type 1
> WARNING: CPU: 0 PID: 3147 at drivers/usb/core/urb.c:494 usb_submit_urb+0xacd/0x1550 drivers/usb/core/urb.c:493
> ...
> Call Trace:
> smsusb_start_streaming+0x16/0x1d0 drivers/media/usb/siano/smsusb.c:195
> smsusb_init_device+0xd85/0x12d0 drivers/media/usb/siano/smsusb.c:475
> smsusb_probe+0x496/0xa90 drivers/media/usb/siano/smsusb.c:566
> usb_probe_interface+0x633/0xb40 drivers/usb/core/driver.c:396
> really_probe+0x3cb/0x1020 drivers/base/dd.c:580
> driver_probe_device+0x178/0x350 drivers/base/dd.c:763
> ...
> hub_event+0x48d/0xd90 drivers/usb/core/hub.c:5644
> process_one_work+0x833/0x10c0 kernel/workqueue.c:2276
> worker_thread+0xac1/0x1300 kernel/workqueue.c:2422
> kthread+0x39a/0x3c0 kernel/kthread.c:313
> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
>
> Reported-and-tested-by: syzbot+12002a39b8c60510f8fb@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 31e0456de5be ("media: usb: siano: Fix general protection fault in smsusb")
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@xxxxxxxxxx>
> ---
> drivers/media/usb/siano/smsusb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
> index 723510520d09..daaac121c670 100644
> --- a/drivers/media/usb/siano/smsusb.c
> +++ b/drivers/media/usb/siano/smsusb.c
> @@ -405,10 +405,10 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
> struct usb_endpoint_descriptor *desc =
> &intf->cur_altsetting->endpoint[i].desc;
>
> - if (desc->bEndpointAddress & USB_DIR_IN) {
> + if (usb_endpoint_is_bulk_in(desc)) {
> dev->in_ep = desc->bEndpointAddress;
> align = usb_endpoint_maxp(desc) - sizeof(struct sms_msg_hdr);
> - } else {
> + } else if (usb_endpoint_is_bulk_out(desc)) {
Did you test it on what devices? I'm not sure if all siano devices
are bulk. Why did you decide to use usb_endpoint_is_bulk_(in|out)
instead of usb_endpoint_dir_(in|out)?
Regards,
Mauro
> dev->out_ep = desc->bEndpointAddress;
> }
> }
>
