* UBSAN: shift-out-of-bounds in option_probe
@ 2020-12-09 7:33 syzbot
2020-12-09 9:24 ` Johan Hovold
2020-12-09 10:42 ` [PATCH] USB: serial: option: add interface-number sanity check to flag handling Johan Hovold
0 siblings, 2 replies; 5+ messages in thread
From: syzbot @ 2020-12-09 7:33 UTC (permalink / raw)
To: gregkh, johan, linux-kernel, linux-usb, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 15ac8fdb Add linux-next specific files for 20201207
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=17dc6adf500000
kernel config: https://syzkaller.appspot.com/x/.config?x=3696b8138207d24d
dashboard link: https://syzkaller.appspot.com/bug?extid=8881b478dad0a7971f79
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12e89613500000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17993623500000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8881b478dad0a7971f79@syzkaller.appspotmail.com
usb 1-1: config 0 interface 109 has no altsetting 0
usb 1-1: New USB device found, idVendor=12d1, idProduct=02cb, bcdDevice= 1.fb
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
================================================================================
UBSAN: shift-out-of-bounds in drivers/usb/serial/option.c:2120:21
shift exponent 109 is too large for 64-bit type 'long unsigned int'
CPU: 0 PID: 3169 Comm: kworker/0:3 Not tainted 5.10.0-rc6-next-20201207-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:79 [inline]
dump_stack+0x107/0x163 lib/dump_stack.c:120
ubsan_epilogue+0xb/0x5a lib/ubsan.c:148
__ubsan_handle_shift_out_of_bounds.cold+0xb1/0x181 lib/ubsan.c:395
option_probe.cold+0x1a/0x1f drivers/usb/serial/option.c:2120
usb_serial_probe+0x32d/0xef0 drivers/usb/serial/usb-serial.c:905
usb_probe_interface+0x315/0x7f0 drivers/usb/core/driver.c:396
really_probe+0x2b1/0xe40 drivers/base/dd.c:554
driver_probe_device+0x285/0x3f0 drivers/base/dd.c:738
__device_attach_driver+0x216/0x2d0 drivers/base/dd.c:844
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: UBSAN: shift-out-of-bounds in option_probe
2020-12-09 7:33 UBSAN: shift-out-of-bounds in option_probe syzbot
@ 2020-12-09 9:24 ` Johan Hovold
2020-12-09 10:42 ` [PATCH] USB: serial: option: add interface-number sanity check to flag handling Johan Hovold
1 sibling, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2020-12-09 9:24 UTC (permalink / raw)
To: syzbot; +Cc: gregkh, johan, linux-kernel, linux-usb, syzkaller-bugs
On Tue, Dec 08, 2020 at 11:33:11PM -0800, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 15ac8fdb Add linux-next specific files for 20201207
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=17dc6adf500000
> kernel config: https://syzkaller.appspot.com/x/.config?x=3696b8138207d24d
> dashboard link: https://syzkaller.appspot.com/bug?extid=8881b478dad0a7971f79
> compiler: gcc (GCC) 10.1.0-syz 20200507
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12e89613500000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17993623500000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+8881b478dad0a7971f79@syzkaller.appspotmail.com
>
> usb 1-1: config 0 interface 109 has no altsetting 0
> usb 1-1: New USB device found, idVendor=12d1, idProduct=02cb, bcdDevice= 1.fb
> usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> usb 1-1: config 0 descriptor??
> ================================================================================
> UBSAN: shift-out-of-bounds in drivers/usb/serial/option.c:2120:21
> shift exponent 109 is too large for 64-bit type 'long unsigned int'
Ok, we need to add a sanity check against large interface numbers before
the device-flag tests. I'll cook something up.
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] USB: serial: option: add interface-number sanity check to flag handling
2020-12-09 7:33 UBSAN: shift-out-of-bounds in option_probe syzbot
2020-12-09 9:24 ` Johan Hovold
@ 2020-12-09 10:42 ` Johan Hovold
2020-12-09 10:52 ` Greg KH
1 sibling, 1 reply; 5+ messages in thread
From: Johan Hovold @ 2020-12-09 10:42 UTC (permalink / raw)
To: linux-usb; +Cc: linux-kernel, Johan Hovold, syzbot+8881b478dad0a7971f79, stable
Add an interface-number sanity check before testing the device flags to
avoid relying on undefined behaviour when left shifting in case a device
uses an interface number greater than or equal to BITS_PER_LONG (i.e. 64
or 32).
Reported-by: syzbot+8881b478dad0a7971f79@syzkaller.appspotmail.com
Fixes: c3a65808f04a ("USB: serial: option: reimplement interface masking")
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/option.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 2a3bfd6f867e..c5908c4f2046 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -561,6 +561,9 @@ static void option_instat_callback(struct urb *urb);
/* Device flags */
+/* Highest interface number which can be used with NCTRL() and RSVD() */
+#define FLAG_IFNUM_MAX 7
+
/* Interface does not support modem-control requests */
#define NCTRL(ifnum) ((BIT(ifnum) & 0xff) << 8)
@@ -2089,6 +2092,14 @@ static struct usb_serial_driver * const serial_drivers[] = {
module_usb_serial_driver(serial_drivers, option_ids);
+static bool iface_is_reserved(unsigned long device_flags, u8 ifnum)
+{
+ if (ifnum > FLAG_IFNUM_MAX)
+ return false;
+
+ return device_flags & RSVD(ifnum);
+}
+
static int option_probe(struct usb_serial *serial,
const struct usb_device_id *id)
{
@@ -2105,7 +2116,7 @@ static int option_probe(struct usb_serial *serial,
* the same class/subclass/protocol as the serial interfaces. Look at
* the Windows driver .INF files for reserved interface numbers.
*/
- if (device_flags & RSVD(iface_desc->bInterfaceNumber))
+ if (iface_is_reserved(device_flags, iface_desc->bInterfaceNumber))
return -ENODEV;
/*
@@ -2121,6 +2132,14 @@ static int option_probe(struct usb_serial *serial,
return 0;
}
+static bool iface_no_modem_control(unsigned long device_flags, u8 ifnum)
+{
+ if (ifnum > FLAG_IFNUM_MAX)
+ return false;
+
+ return device_flags & NCTRL(ifnum);
+}
+
static int option_attach(struct usb_serial *serial)
{
struct usb_interface_descriptor *iface_desc;
@@ -2136,7 +2155,7 @@ static int option_attach(struct usb_serial *serial)
iface_desc = &serial->interface->cur_altsetting->desc;
- if (!(device_flags & NCTRL(iface_desc->bInterfaceNumber)))
+ if (!iface_no_modem_control(device_flags, iface_desc->bInterfaceNumber))
data->use_send_setup = 1;
if (device_flags & ZLP)
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] USB: serial: option: add interface-number sanity check to flag handling
2020-12-09 10:42 ` [PATCH] USB: serial: option: add interface-number sanity check to flag handling Johan Hovold
@ 2020-12-09 10:52 ` Greg KH
2020-12-09 13:57 ` Johan Hovold
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2020-12-09 10:52 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-usb, linux-kernel, syzbot+8881b478dad0a7971f79, stable
On Wed, Dec 09, 2020 at 11:42:21AM +0100, Johan Hovold wrote:
> Add an interface-number sanity check before testing the device flags to
> avoid relying on undefined behaviour when left shifting in case a device
> uses an interface number greater than or equal to BITS_PER_LONG (i.e. 64
> or 32).
>
> Reported-by: syzbot+8881b478dad0a7971f79@syzkaller.appspotmail.com
> Fixes: c3a65808f04a ("USB: serial: option: reimplement interface masking")
> Cc: stable@vger.kernel.org
> Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] USB: serial: option: add interface-number sanity check to flag handling
2020-12-09 10:52 ` Greg KH
@ 2020-12-09 13:57 ` Johan Hovold
0 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2020-12-09 13:57 UTC (permalink / raw)
To: Greg KH
Cc: Johan Hovold, linux-usb, linux-kernel,
syzbot+8881b478dad0a7971f79, stable
On Wed, Dec 09, 2020 at 11:52:45AM +0100, Greg Kroah-Hartman wrote:
> On Wed, Dec 09, 2020 at 11:42:21AM +0100, Johan Hovold wrote:
> > Add an interface-number sanity check before testing the device flags to
> > avoid relying on undefined behaviour when left shifting in case a device
> > uses an interface number greater than or equal to BITS_PER_LONG (i.e. 64
> > or 32).
> >
> > Reported-by: syzbot+8881b478dad0a7971f79@syzkaller.appspotmail.com
> > Fixes: c3a65808f04a ("USB: serial: option: reimplement interface masking")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Johan Hovold <johan@kernel.org>
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Thanks for reviewing. Now applied.
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-09 13:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-09 7:33 UBSAN: shift-out-of-bounds in option_probe syzbot
2020-12-09 9:24 ` Johan Hovold
2020-12-09 10:42 ` [PATCH] USB: serial: option: add interface-number sanity check to flag handling Johan Hovold
2020-12-09 10:52 ` Greg KH
2020-12-09 13:57 ` Johan Hovold
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.