From: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Dongliang Mu <dzm91@hust.edu.cn>,
Andrew Morton <akpm@linux-foundation.org>,
Alan Stern <stern@rowland.harvard.edu>,
<linux-media@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<lvc-project@linuxtesting.org>,
<syzbot+12002a39b8c60510f8fb@syzkaller.appspotmail.com>
Subject: Re: [PATCH] media: usb: siano: fix endpoint checks in smsusb_init_device()
Date: Fri, 3 May 2024 16:58:33 +0100 [thread overview]
Message-ID: <20240503165833.4781fb4a@sal.lan> (raw)
In-Reply-To: <20240409143634.33230-1-n.zhandarovich@fintech.ru>
Em Tue, 9 Apr 2024 07:36:34 -0700
Nikita Zhandarovich <n.zhandarovich@fintech.ru> 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@syzkaller.appspotmail.com
> Fixes: 31e0456de5be ("media: usb: siano: Fix general protection fault in smsusb")
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
> ---
> 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;
> }
> }
>
next prev parent reply other threads:[~2024-05-03 15:58 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 14:36 [PATCH] media: usb: siano: fix endpoint checks in smsusb_init_device() Nikita Zhandarovich
2024-05-03 15:58 ` Mauro Carvalho Chehab [this message]
2024-05-03 16:14 ` Nikita Zhandarovich
2024-05-03 21:20 ` Mauro Carvalho Chehab
2024-05-03 22:52 ` Nikita Zhandarovich
2024-05-06 16:25 ` Nikita Zhandarovich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240503165833.4781fb4a@sal.lan \
--to=mchehab@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=dzm91@hust.edu.cn \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=lvc-project@linuxtesting.org \
--cc=n.zhandarovich@fintech.ru \
--cc=stern@rowland.harvard.edu \
--cc=syzbot+12002a39b8c60510f8fb@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).