* KMSAN: uninit-value in cdc_ncm_set_dgram_size @ 2019-10-30 19:22 syzbot 2019-11-04 21:22 ` Bjørn Mork 2019-11-05 11:11 ` Oliver Neukum 0 siblings, 2 replies; 9+ messages in thread From: syzbot @ 2019-10-30 19:22 UTC (permalink / raw) To: davem, glider, linux-kernel, linux-usb, netdev, oliver, syzkaller-bugs Hello, syzbot found the following crash on: HEAD commit: 96c6c319 net: kasan: kmsan: support CONFIG_GENERIC_CSUM on.. git tree: https://github.com/google/kmsan.git master console output: https://syzkaller.appspot.com/x/log.txt?x=11f103bce00000 kernel config: https://syzkaller.appspot.com/x/.config?x=9e324dfe9c7b0360 dashboard link: https://syzkaller.appspot.com/bug?extid=0631d878823ce2411636 compiler: clang version 9.0.0 (/home/glider/llvm/clang 80fee25776c2fb61e74c1ecb1a523375c2500b69) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10dd9774e00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13651a24e00000 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+0631d878823ce2411636@syzkaller.appspotmail.com ===================================================== BUG: KMSAN: uninit-value in cdc_ncm_set_dgram_size+0x6ba/0xbc0 drivers/net/usb/cdc_ncm.c:587 CPU: 0 PID: 11865 Comm: kworker/0:3 Not tainted 5.4.0-rc5+ #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:77 [inline] dump_stack+0x191/0x1f0 lib/dump_stack.c:113 kmsan_report+0x128/0x220 mm/kmsan/kmsan_report.c:108 __msan_warning+0x73/0xe0 mm/kmsan/kmsan_instr.c:245 cdc_ncm_set_dgram_size+0x6ba/0xbc0 drivers/net/usb/cdc_ncm.c:587 cdc_ncm_setup drivers/net/usb/cdc_ncm.c:673 [inline] cdc_ncm_bind_common+0x2b54/0x3c50 drivers/net/usb/cdc_ncm.c:928 cdc_ncm_bind+0x2de/0x330 drivers/net/usb/cdc_ncm.c:1042 usbnet_probe+0x10d3/0x39d0 drivers/net/usb/usbnet.c:1730 usb_probe_interface+0xd19/0x1310 drivers/usb/core/driver.c:361 really_probe+0xd91/0x1f90 drivers/base/dd.c:552 driver_probe_device+0x1ba/0x510 drivers/base/dd.c:721 __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:828 bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:430 __device_attach+0x489/0x750 drivers/base/dd.c:894 device_initial_probe+0x4a/0x60 drivers/base/dd.c:941 bus_probe_device+0x131/0x390 drivers/base/bus.c:490 device_add+0x25b5/0x2df0 drivers/base/core.c:2202 usb_set_configuration+0x309f/0x3710 drivers/usb/core/message.c:2027 generic_probe+0xe7/0x280 drivers/usb/core/generic.c:210 usb_probe_device+0x146/0x200 drivers/usb/core/driver.c:266 really_probe+0xd91/0x1f90 drivers/base/dd.c:552 driver_probe_device+0x1ba/0x510 drivers/base/dd.c:721 __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:828 bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:430 __device_attach+0x489/0x750 drivers/base/dd.c:894 device_initial_probe+0x4a/0x60 drivers/base/dd.c:941 bus_probe_device+0x131/0x390 drivers/base/bus.c:490 device_add+0x25b5/0x2df0 drivers/base/core.c:2202 usb_new_device+0x23e5/0x2fb0 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x581d/0x72f0 drivers/usb/core/hub.c:5441 process_one_work+0x1572/0x1ef0 kernel/workqueue.c:2269 process_scheduled_works kernel/workqueue.c:2331 [inline] worker_thread+0x189c/0x2460 kernel/workqueue.c:2417 kthread+0x4b5/0x4f0 kernel/kthread.c:256 ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355 Local variable description: ----max_datagram_size@cdc_ncm_set_dgram_size Variable was created at: cdc_ncm_set_dgram_size+0xf5/0xbc0 drivers/net/usb/cdc_ncm.c:564 cdc_ncm_set_dgram_size+0xf5/0xbc0 drivers/net/usb/cdc_ncm.c:564 ===================================================== --- This bug 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 bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: KMSAN: uninit-value in cdc_ncm_set_dgram_size 2019-10-30 19:22 KMSAN: uninit-value in cdc_ncm_set_dgram_size syzbot @ 2019-11-04 21:22 ` Bjørn Mork 2019-11-05 11:15 ` Oliver Neukum 2019-11-05 11:11 ` Oliver Neukum 1 sibling, 1 reply; 9+ messages in thread From: Bjørn Mork @ 2019-11-04 21:22 UTC (permalink / raw) To: syzbot Cc: davem, glider, linux-kernel, linux-usb, netdev, oliver, syzkaller-bugs syzbot <syzbot+0631d878823ce2411636@syzkaller.appspotmail.com> writes: > syzbot found the following crash on: > > HEAD commit: 96c6c319 net: kasan: kmsan: support CONFIG_GENERIC_CSUM on.. > git tree: https://github.com/google/kmsan.git master > console output: https://syzkaller.appspot.com/x/log.txt?x=11f103bce00000 > kernel config: https://syzkaller.appspot.com/x/.config?x=9e324dfe9c7b0360 > dashboard link: https://syzkaller.appspot.com/bug?extid=0631d878823ce2411636 > compiler: clang version 9.0.0 (/home/glider/llvm/clang > 80fee25776c2fb61e74c1ecb1a523375c2500b69) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10dd9774e00000 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13651a24e00000 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+0631d878823ce2411636@syzkaller.appspotmail.com > > ===================================================== > BUG: KMSAN: uninit-value in cdc_ncm_set_dgram_size+0x6ba/0xbc0 > drivers/net/usb/cdc_ncm.c:587 > CPU: 0 PID: 11865 Comm: kworker/0:3 Not tainted 5.4.0-rc5+ #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:77 [inline] > dump_stack+0x191/0x1f0 lib/dump_stack.c:113 > kmsan_report+0x128/0x220 mm/kmsan/kmsan_report.c:108 > __msan_warning+0x73/0xe0 mm/kmsan/kmsan_instr.c:245 > cdc_ncm_set_dgram_size+0x6ba/0xbc0 drivers/net/usb/cdc_ncm.c:587 .. > Variable was created at: > cdc_ncm_set_dgram_size+0xf5/0xbc0 drivers/net/usb/cdc_ncm.c:564 > cdc_ncm_set_dgram_size+0xf5/0xbc0 drivers/net/usb/cdc_ncm.c:564 > ===================================================== This looks like a false positive to me. max_datagram_size is two bytes declared as __le16 max_datagram_size; and the code leading up to the access on drivers/net/usb/cdc_ncm.c:587 is: /* read current mtu value from device */ err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE, USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE, 0, iface_no, &max_datagram_size, 2); if (err < 0) { dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed\n"); goto out; } if (le16_to_cpu(max_datagram_size) == ctx->max_datagram_size) AFAICS, there is no way max_datagram_size can be uninitialized here. usbnet_read_cmd() either read 2 bytes into it or returned an error, causing the access to be skipped. Or am I missing something? I tried to read the syzbot manual to figure out how to tell this to the bot, but couldn't find that either. Not my day today it seems ;-) Please let me know what to do about this. Bjørn ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: KMSAN: uninit-value in cdc_ncm_set_dgram_size 2019-11-04 21:22 ` Bjørn Mork @ 2019-11-05 11:15 ` Oliver Neukum 2019-11-05 12:25 ` Bjørn Mork 0 siblings, 1 reply; 9+ messages in thread From: Oliver Neukum @ 2019-11-05 11:15 UTC (permalink / raw) To: Bjørn Mork, syzbot Cc: davem, glider, linux-kernel, linux-usb, netdev, syzkaller-bugs Am Montag, den 04.11.2019, 22:22 +0100 schrieb Bjørn Mork: > This looks like a false positive to me. max_datagram_size is two bytes > declared as > > __le16 max_datagram_size; > > and the code leading up to the access on drivers/net/usb/cdc_ncm.c:587 > is: > > /* read current mtu value from device */ > err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE, > USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE, > 0, iface_no, &max_datagram_size, 2); At this point err can be 1. > if (err < 0) { > dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed\n"); > goto out; > } > > if (le16_to_cpu(max_datagram_size) == ctx->max_datagram_size) > > > > AFAICS, there is no way max_datagram_size can be uninitialized here. > usbnet_read_cmd() either read 2 bytes into it or returned an error, No. usbnet_read_cmd() will return the number of bytes transfered up to the number requested or an error. > causing the access to be skipped. Or am I missing something? Yes. You can get half the MTU. We have a similar class of bugs with MAC addresses. Regards Oliver ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: KMSAN: uninit-value in cdc_ncm_set_dgram_size 2019-11-05 11:15 ` Oliver Neukum @ 2019-11-05 12:25 ` Bjørn Mork 2019-11-05 13:51 ` Oliver Neukum 2019-11-05 13:55 ` Alexander Potapenko 0 siblings, 2 replies; 9+ messages in thread From: Bjørn Mork @ 2019-11-05 12:25 UTC (permalink / raw) To: Oliver Neukum Cc: syzbot, davem, glider, linux-kernel, linux-usb, netdev, syzkaller-bugs Oliver Neukum <oneukum@suse.com> writes: > Am Montag, den 04.11.2019, 22:22 +0100 schrieb Bjørn Mork: >> This looks like a false positive to me. max_datagram_size is two bytes >> declared as >> >> __le16 max_datagram_size; >> >> and the code leading up to the access on drivers/net/usb/cdc_ncm.c:587 >> is: >> >> /* read current mtu value from device */ >> err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE, >> USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE, >> 0, iface_no, &max_datagram_size, 2); > > At this point err can be 1. > >> if (err < 0) { >> dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed\n"); >> goto out; >> } >> >> if (le16_to_cpu(max_datagram_size) == ctx->max_datagram_size) >> >> >> >> AFAICS, there is no way max_datagram_size can be uninitialized here. >> usbnet_read_cmd() either read 2 bytes into it or returned an error, > > No. usbnet_read_cmd() will return the number of bytes transfered up > to the number requested or an error. Ah, OK. So that could be fixed with e.g. if (err < 2) goto out; Or would it be better to add a strict length checking variant of this API? There are probably lots of similar cases where we expect a multibyte value and a short read is (or should be) considered an error. I can't imagine any situation where we want a 2, 4, 6 or 8 byte value and expect a flexible length returned. >> causing the access to be skipped. Or am I missing something? > > Yes. You can get half the MTU. We have a similar class of bugs > with MAC addresses. Right. And probably all 16 or 32 bit integer reads... Looking at the NCM spec, I see that the wording is annoyingly flexible wrt length - both ways. E.g for GetNetAddress: To get the entire network address, the host should set wLength to at least 6. The function shall never return more than 6 bytes in response to this command. Maybe the correct fix is simply to let usbnet_read_cmd() initialize the full buffer regardless of what the device returns? I.e. diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index dde05e2fdc3e..df3efafca450 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1982,7 +1982,7 @@ static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype, cmd, reqtype, value, index, size); if (size) { - buf = kmalloc(size, GFP_KERNEL); + buf = kzalloc(size, GFP_KERNEL); if (!buf) goto out; } @@ -1992,7 +1992,7 @@ static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype, USB_CTRL_GET_TIMEOUT); if (err > 0 && err <= size) { if (data) - memcpy(data, buf, err); + memcpy(data, buf, size); else netdev_dbg(dev->net, "Huh? Data requested but thrown away.\n"); What do you think? Personally, I don't think it makes sense for a device to return a 1-byte mtu or 3-byte mac address. But the spec allows it and this would at least make it safe. We have a couple of similar bugs elsewhere in the same driver, BTW.. Bjørn ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: KMSAN: uninit-value in cdc_ncm_set_dgram_size 2019-11-05 12:25 ` Bjørn Mork @ 2019-11-05 13:51 ` Oliver Neukum 2019-11-05 13:55 ` Alexander Potapenko 1 sibling, 0 replies; 9+ messages in thread From: Oliver Neukum @ 2019-11-05 13:51 UTC (permalink / raw) To: Bjørn Mork Cc: syzbot, davem, glider, linux-kernel, linux-usb, netdev, syzkaller-bugs Am Dienstag, den 05.11.2019, 13:25 +0100 schrieb Bjørn Mork: > Oliver Neukum <oneukum@suse.com> writes: > > Am Montag, den 04.11.2019, 22:22 +0100 schrieb Bjørn Mork: > Ah, OK. So that could be fixed with e.g. > > if (err < 2) > goto out; > > > Or would it be better to add a strict length checking variant of this > API? There are probably lots of similar cases where we expect a We would lose flexibilty and the check needs to be there anyway. > Right. And probably all 16 or 32 bit integer reads... > > Looking at the NCM spec, I see that the wording is annoyingly flexible > wrt length - both ways. E.g for GetNetAddress: > > To get the entire network address, the host should set wLength to at > least 6. The function shall never return more than 6 bytes in response > to this command. > > Maybe the correct fix is simply to let usbnet_read_cmd() initialize the > full buffer regardless of what the device returns? I.e. This issue has never been observed in the wild. We are defending against a possible attack. It is better to react drastically. > at do you think? > > Personally, I don't think it makes sense for a device to return a 1-byte > mtu or 3-byte mac address. But the spec allows it and this would at > least make it safe. Hence we should ignore such a reply. The support is optional anyway. For usbnet as such, however, we cannot really hardcode the size of a MAC. Regards Oliver ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: KMSAN: uninit-value in cdc_ncm_set_dgram_size 2019-11-05 12:25 ` Bjørn Mork 2019-11-05 13:51 ` Oliver Neukum @ 2019-11-05 13:55 ` Alexander Potapenko 2019-11-05 15:35 ` Greg Kroah-Hartman 1 sibling, 1 reply; 9+ messages in thread From: Alexander Potapenko @ 2019-11-05 13:55 UTC (permalink / raw) To: Bjørn Mork, Greg Kroah-Hartman Cc: Oliver Neukum, syzbot, David Miller, LKML, USB list, Networking, syzkaller-bugs + Greg K-H On Tue, Nov 5, 2019 at 1:25 PM Bjørn Mork <bjorn@mork.no> wrote: > > Oliver Neukum <oneukum@suse.com> writes: > > Am Montag, den 04.11.2019, 22:22 +0100 schrieb Bjørn Mork: > >> This looks like a false positive to me. max_datagram_size is two bytes > >> declared as > >> > >> __le16 max_datagram_size; > >> > >> and the code leading up to the access on drivers/net/usb/cdc_ncm.c:587 > >> is: > >> > >> /* read current mtu value from device */ > >> err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE, > >> USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE, > >> 0, iface_no, &max_datagram_size, 2); > > > > At this point err can be 1. > > > >> if (err < 0) { > >> dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed\n"); > >> goto out; > >> } > >> > >> if (le16_to_cpu(max_datagram_size) == ctx->max_datagram_size) > >> > >> > >> > >> AFAICS, there is no way max_datagram_size can be uninitialized here. > >> usbnet_read_cmd() either read 2 bytes into it or returned an error, > > > > No. usbnet_read_cmd() will return the number of bytes transfered up > > to the number requested or an error. > > Ah, OK. So that could be fixed with e.g. > > if (err < 2) > goto out; It'd better be (err < sizeof(max_datagram_size)), and probably in the call to usbnet_read_cmd() as well. > > Or would it be better to add a strict length checking variant of this > API? There are probably lots of similar cases where we expect a > multibyte value and a short read is (or should be) considered an error. > I can't imagine any situation where we want a 2, 4, 6 or 8 byte value > and expect a flexible length returned. This is really a widespread problem on syzbot: a lot of USB devices use similar code calling usb_control_msg() to read from the device and not checking that the buffer is fully initialized. Greg, do you know how often usb_control_msg() is expected to read less than |size| bytes? Is it viable to make it return an error if this happens? Almost nobody is using this function correctly (i.e. checking that it has read the whole buffer before accessing it). > >> causing the access to be skipped. Or am I missing something? > > > > Yes. You can get half the MTU. We have a similar class of bugs > > with MAC addresses. > > Right. And probably all 16 or 32 bit integer reads... > > Looking at the NCM spec, I see that the wording is annoyingly flexible > wrt length - both ways. E.g for GetNetAddress: > > To get the entire network address, the host should set wLength to at > least 6. The function shall never return more than 6 bytes in response > to this command. > > Maybe the correct fix is simply to let usbnet_read_cmd() initialize the > full buffer regardless of what the device returns? I.e. > > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c > index dde05e2fdc3e..df3efafca450 100644 > --- a/drivers/net/usb/usbnet.c > +++ b/drivers/net/usb/usbnet.c > @@ -1982,7 +1982,7 @@ static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype, > cmd, reqtype, value, index, size); > > if (size) { > - buf = kmalloc(size, GFP_KERNEL); > + buf = kzalloc(size, GFP_KERNEL); > if (!buf) > goto out; > } > @@ -1992,7 +1992,7 @@ static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype, > USB_CTRL_GET_TIMEOUT); > if (err > 0 && err <= size) { > if (data) > - memcpy(data, buf, err); > + memcpy(data, buf, size); > else > netdev_dbg(dev->net, > "Huh? Data requested but thrown away.\n"); > > > > > What do you think? > > Personally, I don't think it makes sense for a device to return a 1-byte > mtu or 3-byte mac address. But the spec allows it and this would at > least make it safe. > > We have a couple of similar bugs elsewhere in the same driver, BTW.. > > > Bjørn -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: KMSAN: uninit-value in cdc_ncm_set_dgram_size 2019-11-05 13:55 ` Alexander Potapenko @ 2019-11-05 15:35 ` Greg Kroah-Hartman 0 siblings, 0 replies; 9+ messages in thread From: Greg Kroah-Hartman @ 2019-11-05 15:35 UTC (permalink / raw) To: Alexander Potapenko Cc: Bjørn Mork, Oliver Neukum, syzbot, David Miller, LKML, USB list, Networking, syzkaller-bugs On Tue, Nov 05, 2019 at 02:55:09PM +0100, Alexander Potapenko wrote: > + Greg K-H > On Tue, Nov 5, 2019 at 1:25 PM Bjørn Mork <bjorn@mork.no> wrote: > > > > Oliver Neukum <oneukum@suse.com> writes: > > > Am Montag, den 04.11.2019, 22:22 +0100 schrieb Bjørn Mork: > > >> This looks like a false positive to me. max_datagram_size is two bytes > > >> declared as > > >> > > >> __le16 max_datagram_size; > > >> > > >> and the code leading up to the access on drivers/net/usb/cdc_ncm.c:587 > > >> is: > > >> > > >> /* read current mtu value from device */ > > >> err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE, > > >> USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE, > > >> 0, iface_no, &max_datagram_size, 2); > > > > > > At this point err can be 1. > > > > > >> if (err < 0) { > > >> dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed\n"); > > >> goto out; > > >> } > > >> > > >> if (le16_to_cpu(max_datagram_size) == ctx->max_datagram_size) > > >> > > >> > > >> > > >> AFAICS, there is no way max_datagram_size can be uninitialized here. > > >> usbnet_read_cmd() either read 2 bytes into it or returned an error, > > > > > > No. usbnet_read_cmd() will return the number of bytes transfered up > > > to the number requested or an error. > > > > Ah, OK. So that could be fixed with e.g. > > > > if (err < 2) > > goto out; > It'd better be (err < sizeof(max_datagram_size)), and probably in the > call to usbnet_read_cmd() as well. > > > > Or would it be better to add a strict length checking variant of this > > API? There are probably lots of similar cases where we expect a > > multibyte value and a short read is (or should be) considered an error. > > I can't imagine any situation where we want a 2, 4, 6 or 8 byte value > > and expect a flexible length returned. > This is really a widespread problem on syzbot: a lot of USB devices > use similar code calling usb_control_msg() to read from the device and > not checking that the buffer is fully initialized. > > Greg, do you know how often usb_control_msg() is expected to read less > than |size| bytes? Is it viable to make it return an error if this > happens? > Almost nobody is using this function correctly (i.e. checking that it > has read the whole buffer before accessing it). It could return less than size if the endpoint size isn't the same as the message. I think. It's been a long time since I've read the USB spec in that area, but usually if the size is "short" then status should also be set saying something went wrong, right? Ah wait, you are playing the "malicious device" card, I think we always just need to check the thing :( sorry, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: KMSAN: uninit-value in cdc_ncm_set_dgram_size 2019-10-30 19:22 KMSAN: uninit-value in cdc_ncm_set_dgram_size syzbot 2019-11-04 21:22 ` Bjørn Mork @ 2019-11-05 11:11 ` Oliver Neukum 2019-11-05 12:51 ` syzbot 1 sibling, 1 reply; 9+ messages in thread From: Oliver Neukum @ 2019-11-05 11:11 UTC (permalink / raw) To: syzbot, davem, glider, linux-kernel, linux-usb, netdev, syzkaller-bugs [-- Attachment #1: Type: text/plain, Size: 948 bytes --] Am Mittwoch, den 30.10.2019, 12:22 -0700 schrieb syzbot: > Hello, > > syzbot found the following crash on: > > HEAD commit: 96c6c319 net: kasan: kmsan: support CONFIG_GENERIC_CSUM on.. > git tree: https://github.com/google/kmsan.git master > console output: https://syzkaller.appspot.com/x/log.txt?x=11f103bce00000 > kernel config: https://syzkaller.appspot.com/x/.config?x=9e324dfe9c7b0360 > dashboard link: https://syzkaller.appspot.com/bug?extid=0631d878823ce2411636 > compiler: clang version 9.0.0 (/home/glider/llvm/clang > 80fee25776c2fb61e74c1ecb1a523375c2500b69) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10dd9774e00000 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13651a24e00000 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+0631d878823ce2411636@syzkaller.appspotmail.com #syz test: https://github.com/google/kmsan.git 96c6c319 [-- Attachment #2: 0001-CDC-NCM-handle-incomplete-transfer-of-MTU.patch --] [-- Type: text/x-patch, Size: 1146 bytes --] From 090ac0305bb47da9336c0188e0e59e50ff2243c3 Mon Sep 17 00:00:00 2001 From: Oliver Neukum <oneukum@suse.com> Date: Tue, 5 Nov 2019 12:04:44 +0100 Subject: [PATCH] CDC-NCM: handle incomplete transfer of MTU A malicious device may give half an answer when asked for its MTU. The driver will proceed after this with a garbage MTU. Anything but a complete answer must be treated as an error. Reported-by: syzbot+0631d878823ce2411636@syzkaller.appspotmail.com Signed-off-by: Oliver Neukum <oneukum@suse.com> --- drivers/net/usb/cdc_ncm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 00cab3f43a4c..939487a5f4bc 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -579,7 +579,7 @@ static void cdc_ncm_set_dgram_size(struct usbnet *dev, int new_size) err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE, USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE, 0, iface_no, &max_datagram_size, 2); - if (err < 0) { + if (err < 2) { dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed\n"); goto out; } -- 2.16.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: KMSAN: uninit-value in cdc_ncm_set_dgram_size 2019-11-05 11:11 ` Oliver Neukum @ 2019-11-05 12:51 ` syzbot 0 siblings, 0 replies; 9+ messages in thread From: syzbot @ 2019-11-05 12:51 UTC (permalink / raw) To: davem, glider, linux-kernel, linux-usb, netdev, oneukum, syzkaller-bugs Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+0631d878823ce2411636@syzkaller.appspotmail.com Tested on: commit: 96c6c319 net: kasan: kmsan: support CONFIG_GENERIC_CSUM on.. git tree: https://github.com/google/kmsan.git kernel config: https://syzkaller.appspot.com/x/.config?x=9e324dfe9c7b0360 dashboard link: https://syzkaller.appspot.com/bug?extid=0631d878823ce2411636 compiler: clang version 9.0.0 (/home/glider/llvm/clang 80fee25776c2fb61e74c1ecb1a523375c2500b69) patch: https://syzkaller.appspot.com/x/patch.diff?x=16b798dce00000 Note: testing is done by a robot and is best-effort only. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-11-05 15:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-30 19:22 KMSAN: uninit-value in cdc_ncm_set_dgram_size syzbot 2019-11-04 21:22 ` Bjørn Mork 2019-11-05 11:15 ` Oliver Neukum 2019-11-05 12:25 ` Bjørn Mork 2019-11-05 13:51 ` Oliver Neukum 2019-11-05 13:55 ` Alexander Potapenko 2019-11-05 15:35 ` Greg Kroah-Hartman 2019-11-05 11:11 ` Oliver Neukum 2019-11-05 12:51 ` syzbot
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 NNTP newsgroup(s).