Re: net/gadget: slab-out-of-bounds write in dev_config
Hi, Alan Sternwrites: >> > Index: usb-4.x/drivers/usb/gadget/legacy/inode.c >> > === >> > --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c >> > +++ usb-4.x/drivers/usb/gadget/legacy/inode.c >> > @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ >> >/* data and/or status stage for control request */ >> >} else if (dev->state == STATE_DEV_SETUP) { >> > >> > - /* IN DATA+STATUS caller makes len <= wLength */ >> > + len = min(len, (size_t) dev->setup_wLength); >> >if (dev->setup_in) { >> >retval = setup_req (dev->gadget->ep0, dev->req, len); >> >if (retval == 0) { >> > >> >> I already have a patch from Greg for this. See [1] >> >> [1] >> https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?id=230bc0cb8ff222d9f0fbbd93a80393140b39481f > > The two patches fix different problems. My patch goes on the pathway > where dev->state > STATE_DEV_OPENED in dev_config(), and Greg's patch > handles the case where it is <=. Okay, here's what I have so far in my testing/fixes: $ git --no-pager shortlog testing/fixes ^linus/master Alan Stern (5): USB: dummy-hcd: fix bug in stop_activity (handle ep0) USB: gadgetfs: fix unbounded memory allocation bug USB: gadgetfs: fix use-after-free bug USB: gadgetfs: fix checks of wTotalLength in config descriptors USB: gadgetfs: remove unnecessary assignment Baolin Wang (1): usb: gadget: f_fs: Fix possibe deadlock Felipe Balbi (4): usb: dwc3: ep0: add dwc3_ep0_prepare_one_trb() usb: dwc3: ep0: explicitly call dwc3_ep0_prepare_one_trb() usb: dwc3: gadget: always unmap EP0 requests usb: dwc3: core: avoid Overflow events Greg Kroah-Hartman (1): usb: gadgetfs: restrict upper bound on device configuration size Grygorii Strashko (1): usb: dwc3: omap: fix race of pm runtime with irq handler in probe Hans de Goede (1): usb: dwc3: pci: Fix dr_mode misspelling Heikki Krogerus (1): usb: dwc3: pci: add Intel Gemini Lake PCI ID Janusz Dziedzic (1): usb: dwc3: skip interrupt when ep disabled John Youn (1): usb: dwc3: pci: Add "linux,sysdev_is_parent" property Krzysztof Opasiak (1): usb: gadget: composite: Test get_alt() presence instead of set_alt() Marek Szyprowski (1): usb: dwc2: fix flags for DMA descriptor allocation in dwc2_hsotg_ep_enable Stefan Wahren (4): usb: dwc2: Do not set host parameter in peripheral mode usb: dwc2: fix dwc2_get_device_property for u8 and u16 usb: dwc2: fix default value for DMA support usb: dwc2: gadget: fix default value for gadget-dma-desc Vincent Pelletier (2): usb: gadget: f_fs: Document eventfd effect on descriptor format. usb: gadget: f_fs: Fix ExtCompat descriptor validation -- balbi signature.asc Description: PGP signature
Re: net/gadget: slab-out-of-bounds write in dev_config
Hi, Alan Stern writes: >> > Index: usb-4.x/drivers/usb/gadget/legacy/inode.c >> > === >> > --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c >> > +++ usb-4.x/drivers/usb/gadget/legacy/inode.c >> > @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ >> >/* data and/or status stage for control request */ >> >} else if (dev->state == STATE_DEV_SETUP) { >> > >> > - /* IN DATA+STATUS caller makes len <= wLength */ >> > + len = min(len, (size_t) dev->setup_wLength); >> >if (dev->setup_in) { >> >retval = setup_req (dev->gadget->ep0, dev->req, len); >> >if (retval == 0) { >> > >> >> I already have a patch from Greg for this. See [1] >> >> [1] >> https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?id=230bc0cb8ff222d9f0fbbd93a80393140b39481f > > The two patches fix different problems. My patch goes on the pathway > where dev->state > STATE_DEV_OPENED in dev_config(), and Greg's patch > handles the case where it is <=. Okay, here's what I have so far in my testing/fixes: $ git --no-pager shortlog testing/fixes ^linus/master Alan Stern (5): USB: dummy-hcd: fix bug in stop_activity (handle ep0) USB: gadgetfs: fix unbounded memory allocation bug USB: gadgetfs: fix use-after-free bug USB: gadgetfs: fix checks of wTotalLength in config descriptors USB: gadgetfs: remove unnecessary assignment Baolin Wang (1): usb: gadget: f_fs: Fix possibe deadlock Felipe Balbi (4): usb: dwc3: ep0: add dwc3_ep0_prepare_one_trb() usb: dwc3: ep0: explicitly call dwc3_ep0_prepare_one_trb() usb: dwc3: gadget: always unmap EP0 requests usb: dwc3: core: avoid Overflow events Greg Kroah-Hartman (1): usb: gadgetfs: restrict upper bound on device configuration size Grygorii Strashko (1): usb: dwc3: omap: fix race of pm runtime with irq handler in probe Hans de Goede (1): usb: dwc3: pci: Fix dr_mode misspelling Heikki Krogerus (1): usb: dwc3: pci: add Intel Gemini Lake PCI ID Janusz Dziedzic (1): usb: dwc3: skip interrupt when ep disabled John Youn (1): usb: dwc3: pci: Add "linux,sysdev_is_parent" property Krzysztof Opasiak (1): usb: gadget: composite: Test get_alt() presence instead of set_alt() Marek Szyprowski (1): usb: dwc2: fix flags for DMA descriptor allocation in dwc2_hsotg_ep_enable Stefan Wahren (4): usb: dwc2: Do not set host parameter in peripheral mode usb: dwc2: fix dwc2_get_device_property for u8 and u16 usb: dwc2: fix default value for DMA support usb: dwc2: gadget: fix default value for gadget-dma-desc Vincent Pelletier (2): usb: gadget: f_fs: Document eventfd effect on descriptor format. usb: gadget: f_fs: Fix ExtCompat descriptor validation -- balbi signature.asc Description: PGP signature
Re: net/gadget: slab-out-of-bounds write in dev_config
On Tue, 27 Dec 2016, Felipe Balbi wrote: > > Hi, > > Alan Sternwrites: > > On Tue, 6 Dec 2016, Andrey Konovalov wrote: > > > >> Hi! > >> > >> I've got the following error report while running the syzkaller fuzzer. > >> > >> ep0_write() doesn't check the length, so a user can cause an > >> out-of-bounds with both size and data controlled. > >> There's a comment which says "IN DATA+STATUS caller makes len <= > >> wLength". While I'm not exactly sure what that means, the length seems > >> to be passed unmodified directly from dev_config(). > > > > You're right about the comment being misleading. It looks like > > somebody forgot to actually do the check. > > > >> This doesn't seem to be a critical security issue since gadgetfs can't > >> be mounted from a user namespace. > >> > >> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). > >> > >> == > >> BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr > >> 88003c47e160 > >> Write of size 65537 by task syz-executor0/6356 > >> CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs > >> 01/01/2011 > >> 88003c107ad8 81f96aba 3dc11ef0 110007820eee > >> ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 > >> 81f96828 813fb4a0 88003b6eadc0 88003c107738 > >> Call Trace: > >> [< inline >] __dump_stack lib/dump_stack.c:15 > >> [] dump_stack+0x292/0x398 lib/dump_stack.c:51 > >> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 > >> [< inline >] print_address_description mm/kasan/report.c:197 > >> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 > >> [] kasan_report+0x35/0x40 mm/kasan/report.c:306 > >> [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 > >> [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 > >> [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 > >> [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 > >> [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 > >> [] dev_config+0x86f/0x1190 > >> drivers/usb/gadget/legacy/inode.c:1759 > >> [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 > >> [] vfs_write+0x170/0x4e0 fs/read_write.c:560 > >> [< inline >] SYSC_write fs/read_write.c:607 > >> [] SyS_write+0xfb/0x230 fs/read_write.c:599 > >> [] entry_SYSCALL_64_fastpath+0x1f/0xc2 > > > > How does this patch work out? > > > > Alan Stern > > > > > > > > Index: usb-4.x/drivers/usb/gadget/legacy/inode.c > > === > > --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c > > +++ usb-4.x/drivers/usb/gadget/legacy/inode.c > > @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ > > /* data and/or status stage for control request */ > > } else if (dev->state == STATE_DEV_SETUP) { > > > > - /* IN DATA+STATUS caller makes len <= wLength */ > > + len = min(len, (size_t) dev->setup_wLength); > > if (dev->setup_in) { > > retval = setup_req (dev->gadget->ep0, dev->req, len); > > if (retval == 0) { > > > > I already have a patch from Greg for this. See [1] > > [1] > https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?id=230bc0cb8ff222d9f0fbbd93a80393140b39481f The two patches fix different problems. My patch goes on the pathway where dev->state > STATE_DEV_OPENED in dev_config(), and Greg's patch handles the case where it is <=. Alan Stern
Re: net/gadget: slab-out-of-bounds write in dev_config
On Tue, 27 Dec 2016, Felipe Balbi wrote: > > Hi, > > Alan Stern writes: > > On Tue, 6 Dec 2016, Andrey Konovalov wrote: > > > >> Hi! > >> > >> I've got the following error report while running the syzkaller fuzzer. > >> > >> ep0_write() doesn't check the length, so a user can cause an > >> out-of-bounds with both size and data controlled. > >> There's a comment which says "IN DATA+STATUS caller makes len <= > >> wLength". While I'm not exactly sure what that means, the length seems > >> to be passed unmodified directly from dev_config(). > > > > You're right about the comment being misleading. It looks like > > somebody forgot to actually do the check. > > > >> This doesn't seem to be a critical security issue since gadgetfs can't > >> be mounted from a user namespace. > >> > >> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). > >> > >> == > >> BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr > >> 88003c47e160 > >> Write of size 65537 by task syz-executor0/6356 > >> CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs > >> 01/01/2011 > >> 88003c107ad8 81f96aba 3dc11ef0 110007820eee > >> ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 > >> 81f96828 813fb4a0 88003b6eadc0 88003c107738 > >> Call Trace: > >> [< inline >] __dump_stack lib/dump_stack.c:15 > >> [] dump_stack+0x292/0x398 lib/dump_stack.c:51 > >> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 > >> [< inline >] print_address_description mm/kasan/report.c:197 > >> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 > >> [] kasan_report+0x35/0x40 mm/kasan/report.c:306 > >> [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 > >> [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 > >> [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 > >> [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 > >> [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 > >> [] dev_config+0x86f/0x1190 > >> drivers/usb/gadget/legacy/inode.c:1759 > >> [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 > >> [] vfs_write+0x170/0x4e0 fs/read_write.c:560 > >> [< inline >] SYSC_write fs/read_write.c:607 > >> [] SyS_write+0xfb/0x230 fs/read_write.c:599 > >> [] entry_SYSCALL_64_fastpath+0x1f/0xc2 > > > > How does this patch work out? > > > > Alan Stern > > > > > > > > Index: usb-4.x/drivers/usb/gadget/legacy/inode.c > > === > > --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c > > +++ usb-4.x/drivers/usb/gadget/legacy/inode.c > > @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ > > /* data and/or status stage for control request */ > > } else if (dev->state == STATE_DEV_SETUP) { > > > > - /* IN DATA+STATUS caller makes len <= wLength */ > > + len = min(len, (size_t) dev->setup_wLength); > > if (dev->setup_in) { > > retval = setup_req (dev->gadget->ep0, dev->req, len); > > if (retval == 0) { > > > > I already have a patch from Greg for this. See [1] > > [1] > https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?id=230bc0cb8ff222d9f0fbbd93a80393140b39481f The two patches fix different problems. My patch goes on the pathway where dev->state > STATE_DEV_OPENED in dev_config(), and Greg's patch handles the case where it is <=. Alan Stern
Re: net/gadget: slab-out-of-bounds write in dev_config
Hi, Alan Sternwrites: > On Tue, 6 Dec 2016, Andrey Konovalov wrote: > >> Hi! >> >> I've got the following error report while running the syzkaller fuzzer. >> >> ep0_write() doesn't check the length, so a user can cause an >> out-of-bounds with both size and data controlled. >> There's a comment which says "IN DATA+STATUS caller makes len <= >> wLength". While I'm not exactly sure what that means, the length seems >> to be passed unmodified directly from dev_config(). > > You're right about the comment being misleading. It looks like > somebody forgot to actually do the check. > >> This doesn't seem to be a critical security issue since gadgetfs can't >> be mounted from a user namespace. >> >> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). >> >> == >> BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr >> 88003c47e160 >> Write of size 65537 by task syz-executor0/6356 >> CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> 88003c107ad8 81f96aba 3dc11ef0 110007820eee >> ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 >> 81f96828 813fb4a0 88003b6eadc0 88003c107738 >> Call Trace: >> [< inline >] __dump_stack lib/dump_stack.c:15 >> [] dump_stack+0x292/0x398 lib/dump_stack.c:51 >> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 >> [< inline >] print_address_description mm/kasan/report.c:197 >> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 >> [] kasan_report+0x35/0x40 mm/kasan/report.c:306 >> [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 >> [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 >> [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 >> [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 >> [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 >> [] dev_config+0x86f/0x1190 >> drivers/usb/gadget/legacy/inode.c:1759 >> [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 >> [] vfs_write+0x170/0x4e0 fs/read_write.c:560 >> [< inline >] SYSC_write fs/read_write.c:607 >> [] SyS_write+0xfb/0x230 fs/read_write.c:599 >> [] entry_SYSCALL_64_fastpath+0x1f/0xc2 > > How does this patch work out? > > Alan Stern > > > > Index: usb-4.x/drivers/usb/gadget/legacy/inode.c > === > --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c > +++ usb-4.x/drivers/usb/gadget/legacy/inode.c > @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ > /* data and/or status stage for control request */ > } else if (dev->state == STATE_DEV_SETUP) { > > - /* IN DATA+STATUS caller makes len <= wLength */ > + len = min(len, (size_t) dev->setup_wLength); > if (dev->setup_in) { > retval = setup_req (dev->gadget->ep0, dev->req, len); > if (retval == 0) { > I already have a patch from Greg for this. See [1] [1] https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?id=230bc0cb8ff222d9f0fbbd93a80393140b39481f -- balbi signature.asc Description: PGP signature
Re: net/gadget: slab-out-of-bounds write in dev_config
Hi, Alan Stern writes: > On Tue, 6 Dec 2016, Andrey Konovalov wrote: > >> Hi! >> >> I've got the following error report while running the syzkaller fuzzer. >> >> ep0_write() doesn't check the length, so a user can cause an >> out-of-bounds with both size and data controlled. >> There's a comment which says "IN DATA+STATUS caller makes len <= >> wLength". While I'm not exactly sure what that means, the length seems >> to be passed unmodified directly from dev_config(). > > You're right about the comment being misleading. It looks like > somebody forgot to actually do the check. > >> This doesn't seem to be a critical security issue since gadgetfs can't >> be mounted from a user namespace. >> >> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). >> >> == >> BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr >> 88003c47e160 >> Write of size 65537 by task syz-executor0/6356 >> CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> 88003c107ad8 81f96aba 3dc11ef0 110007820eee >> ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 >> 81f96828 813fb4a0 88003b6eadc0 88003c107738 >> Call Trace: >> [< inline >] __dump_stack lib/dump_stack.c:15 >> [] dump_stack+0x292/0x398 lib/dump_stack.c:51 >> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 >> [< inline >] print_address_description mm/kasan/report.c:197 >> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 >> [] kasan_report+0x35/0x40 mm/kasan/report.c:306 >> [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 >> [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 >> [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 >> [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 >> [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 >> [] dev_config+0x86f/0x1190 >> drivers/usb/gadget/legacy/inode.c:1759 >> [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 >> [] vfs_write+0x170/0x4e0 fs/read_write.c:560 >> [< inline >] SYSC_write fs/read_write.c:607 >> [] SyS_write+0xfb/0x230 fs/read_write.c:599 >> [] entry_SYSCALL_64_fastpath+0x1f/0xc2 > > How does this patch work out? > > Alan Stern > > > > Index: usb-4.x/drivers/usb/gadget/legacy/inode.c > === > --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c > +++ usb-4.x/drivers/usb/gadget/legacy/inode.c > @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ > /* data and/or status stage for control request */ > } else if (dev->state == STATE_DEV_SETUP) { > > - /* IN DATA+STATUS caller makes len <= wLength */ > + len = min(len, (size_t) dev->setup_wLength); > if (dev->setup_in) { > retval = setup_req (dev->gadget->ep0, dev->req, len); > if (retval == 0) { > I already have a patch from Greg for this. See [1] [1] https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?id=230bc0cb8ff222d9f0fbbd93a80393140b39481f -- balbi signature.asc Description: PGP signature
Re: net/gadget: slab-out-of-bounds write in dev_config
On Tue, Dec 6, 2016 at 4:30 PM, Alan Sternwrote: > On Tue, 6 Dec 2016, Andrey Konovalov wrote: > >> Hi! >> >> I've got the following error report while running the syzkaller fuzzer. >> >> ep0_write() doesn't check the length, so a user can cause an >> out-of-bounds with both size and data controlled. >> There's a comment which says "IN DATA+STATUS caller makes len <= >> wLength". While I'm not exactly sure what that means, the length seems >> to be passed unmodified directly from dev_config(). > > You're right about the comment being misleading. It looks like > somebody forgot to actually do the check. > >> This doesn't seem to be a critical security issue since gadgetfs can't >> be mounted from a user namespace. >> >> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). >> >> == >> BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr >> 88003c47e160 >> Write of size 65537 by task syz-executor0/6356 >> CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> 88003c107ad8 81f96aba 3dc11ef0 110007820eee >> ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 >> 81f96828 813fb4a0 88003b6eadc0 88003c107738 >> Call Trace: >> [< inline >] __dump_stack lib/dump_stack.c:15 >> [] dump_stack+0x292/0x398 lib/dump_stack.c:51 >> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 >> [< inline >] print_address_description mm/kasan/report.c:197 >> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 >> [] kasan_report+0x35/0x40 mm/kasan/report.c:306 >> [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 >> [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 >> [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 >> [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 >> [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 >> [] dev_config+0x86f/0x1190 >> drivers/usb/gadget/legacy/inode.c:1759 >> [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 >> [] vfs_write+0x170/0x4e0 fs/read_write.c:560 >> [< inline >] SYSC_write fs/read_write.c:607 >> [] SyS_write+0xfb/0x230 fs/read_write.c:599 >> [] entry_SYSCALL_64_fastpath+0x1f/0xc2 > > How does this patch work out? I believe it fixes the issue. Thanks! > > Alan Stern > > > > Index: usb-4.x/drivers/usb/gadget/legacy/inode.c > === > --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c > +++ usb-4.x/drivers/usb/gadget/legacy/inode.c > @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ > /* data and/or status stage for control request */ > } else if (dev->state == STATE_DEV_SETUP) { > > - /* IN DATA+STATUS caller makes len <= wLength */ > + len = min(len, (size_t) dev->setup_wLength); > if (dev->setup_in) { > retval = setup_req (dev->gadget->ep0, dev->req, len); > if (retval == 0) { >
Re: net/gadget: slab-out-of-bounds write in dev_config
On Tue, Dec 6, 2016 at 4:30 PM, Alan Stern wrote: > On Tue, 6 Dec 2016, Andrey Konovalov wrote: > >> Hi! >> >> I've got the following error report while running the syzkaller fuzzer. >> >> ep0_write() doesn't check the length, so a user can cause an >> out-of-bounds with both size and data controlled. >> There's a comment which says "IN DATA+STATUS caller makes len <= >> wLength". While I'm not exactly sure what that means, the length seems >> to be passed unmodified directly from dev_config(). > > You're right about the comment being misleading. It looks like > somebody forgot to actually do the check. > >> This doesn't seem to be a critical security issue since gadgetfs can't >> be mounted from a user namespace. >> >> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). >> >> == >> BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr >> 88003c47e160 >> Write of size 65537 by task syz-executor0/6356 >> CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> 88003c107ad8 81f96aba 3dc11ef0 110007820eee >> ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 >> 81f96828 813fb4a0 88003b6eadc0 88003c107738 >> Call Trace: >> [< inline >] __dump_stack lib/dump_stack.c:15 >> [] dump_stack+0x292/0x398 lib/dump_stack.c:51 >> [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 >> [< inline >] print_address_description mm/kasan/report.c:197 >> [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 >> [] kasan_report+0x35/0x40 mm/kasan/report.c:306 >> [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 >> [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 >> [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 >> [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 >> [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 >> [] dev_config+0x86f/0x1190 >> drivers/usb/gadget/legacy/inode.c:1759 >> [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 >> [] vfs_write+0x170/0x4e0 fs/read_write.c:560 >> [< inline >] SYSC_write fs/read_write.c:607 >> [] SyS_write+0xfb/0x230 fs/read_write.c:599 >> [] entry_SYSCALL_64_fastpath+0x1f/0xc2 > > How does this patch work out? I believe it fixes the issue. Thanks! > > Alan Stern > > > > Index: usb-4.x/drivers/usb/gadget/legacy/inode.c > === > --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c > +++ usb-4.x/drivers/usb/gadget/legacy/inode.c > @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ > /* data and/or status stage for control request */ > } else if (dev->state == STATE_DEV_SETUP) { > > - /* IN DATA+STATUS caller makes len <= wLength */ > + len = min(len, (size_t) dev->setup_wLength); > if (dev->setup_in) { > retval = setup_req (dev->gadget->ep0, dev->req, len); > if (retval == 0) { >
Re: net/gadget: slab-out-of-bounds write in dev_config
On Tue, 6 Dec 2016, Andrey Konovalov wrote: > Hi! > > I've got the following error report while running the syzkaller fuzzer. > > ep0_write() doesn't check the length, so a user can cause an > out-of-bounds with both size and data controlled. > There's a comment which says "IN DATA+STATUS caller makes len <= > wLength". While I'm not exactly sure what that means, the length seems > to be passed unmodified directly from dev_config(). You're right about the comment being misleading. It looks like somebody forgot to actually do the check. > This doesn't seem to be a critical security issue since gadgetfs can't > be mounted from a user namespace. > > On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). > > == > BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr > 88003c47e160 > Write of size 65537 by task syz-executor0/6356 > CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > 88003c107ad8 81f96aba 3dc11ef0 110007820eee > ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 > 81f96828 813fb4a0 88003b6eadc0 88003c107738 > Call Trace: > [< inline >] __dump_stack lib/dump_stack.c:15 > [] dump_stack+0x292/0x398 lib/dump_stack.c:51 > [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 > [< inline >] print_address_description mm/kasan/report.c:197 > [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 > [] kasan_report+0x35/0x40 mm/kasan/report.c:306 > [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 > [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 > [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 > [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 > [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 > [] dev_config+0x86f/0x1190 > drivers/usb/gadget/legacy/inode.c:1759 > [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 > [] vfs_write+0x170/0x4e0 fs/read_write.c:560 > [< inline >] SYSC_write fs/read_write.c:607 > [] SyS_write+0xfb/0x230 fs/read_write.c:599 > [] entry_SYSCALL_64_fastpath+0x1f/0xc2 How does this patch work out? Alan Stern Index: usb-4.x/drivers/usb/gadget/legacy/inode.c === --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c +++ usb-4.x/drivers/usb/gadget/legacy/inode.c @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ /* data and/or status stage for control request */ } else if (dev->state == STATE_DEV_SETUP) { - /* IN DATA+STATUS caller makes len <= wLength */ + len = min(len, (size_t) dev->setup_wLength); if (dev->setup_in) { retval = setup_req (dev->gadget->ep0, dev->req, len); if (retval == 0) {
Re: net/gadget: slab-out-of-bounds write in dev_config
On Tue, 6 Dec 2016, Andrey Konovalov wrote: > Hi! > > I've got the following error report while running the syzkaller fuzzer. > > ep0_write() doesn't check the length, so a user can cause an > out-of-bounds with both size and data controlled. > There's a comment which says "IN DATA+STATUS caller makes len <= > wLength". While I'm not exactly sure what that means, the length seems > to be passed unmodified directly from dev_config(). You're right about the comment being misleading. It looks like somebody forgot to actually do the check. > This doesn't seem to be a critical security issue since gadgetfs can't > be mounted from a user namespace. > > On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). > > == > BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr > 88003c47e160 > Write of size 65537 by task syz-executor0/6356 > CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > 88003c107ad8 81f96aba 3dc11ef0 110007820eee > ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 > 81f96828 813fb4a0 88003b6eadc0 88003c107738 > Call Trace: > [< inline >] __dump_stack lib/dump_stack.c:15 > [] dump_stack+0x292/0x398 lib/dump_stack.c:51 > [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 > [< inline >] print_address_description mm/kasan/report.c:197 > [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 > [] kasan_report+0x35/0x40 mm/kasan/report.c:306 > [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 > [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 > [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 > [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 > [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 > [] dev_config+0x86f/0x1190 > drivers/usb/gadget/legacy/inode.c:1759 > [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 > [] vfs_write+0x170/0x4e0 fs/read_write.c:560 > [< inline >] SYSC_write fs/read_write.c:607 > [] SyS_write+0xfb/0x230 fs/read_write.c:599 > [] entry_SYSCALL_64_fastpath+0x1f/0xc2 How does this patch work out? Alan Stern Index: usb-4.x/drivers/usb/gadget/legacy/inode.c === --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c +++ usb-4.x/drivers/usb/gadget/legacy/inode.c @@ -1126,7 +1126,7 @@ ep0_write (struct file *fd, const char _ /* data and/or status stage for control request */ } else if (dev->state == STATE_DEV_SETUP) { - /* IN DATA+STATUS caller makes len <= wLength */ + len = min(len, (size_t) dev->setup_wLength); if (dev->setup_in) { retval = setup_req (dev->gadget->ep0, dev->req, len); if (retval == 0) {
net/gadget: slab-out-of-bounds write in dev_config
Hi! I've got the following error report while running the syzkaller fuzzer. ep0_write() doesn't check the length, so a user can cause an out-of-bounds with both size and data controlled. There's a comment which says "IN DATA+STATUS caller makes len <= wLength". While I'm not exactly sure what that means, the length seems to be passed unmodified directly from dev_config(). This doesn't seem to be a critical security issue since gadgetfs can't be mounted from a user namespace. On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). == BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr 88003c47e160 Write of size 65537 by task syz-executor0/6356 CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 88003c107ad8 81f96aba 3dc11ef0 110007820eee ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 81f96828 813fb4a0 88003b6eadc0 88003c107738 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x292/0x398 lib/dump_stack.c:51 [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 [< inline >] print_address_description mm/kasan/report.c:197 [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 [] kasan_report+0x35/0x40 mm/kasan/report.c:306 [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 [] dev_config+0x86f/0x1190 drivers/usb/gadget/legacy/inode.c:1759 [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 [] vfs_write+0x170/0x4e0 fs/read_write.c:560 [< inline >] SYSC_write fs/read_write.c:607 [] SyS_write+0xfb/0x230 fs/read_write.c:599 [] entry_SYSCALL_64_fastpath+0x1f/0xc2 Object at 88003c47e038, in cache kmalloc-1024 size: 1024 Allocated: PID = 4565 [ 43.417154] [] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57 [ 43.417154] [] save_stack+0x43/0xd0 mm/kasan/kasan.c:495 [ 43.417154] [< inline >] set_track mm/kasan/kasan.c:507 [ 43.417154] [] kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:598 [ 43.417154] [] kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2735 [ 43.417154] [< inline >] kmalloc include/linux/slab.h:490 [ 43.417154] [< inline >] kzalloc include/linux/slab.h:636 [ 43.417154] [< inline >] dev_new drivers/usb/gadget/legacy/inode.c:170 [ 43.417154] [] gadgetfs_fill_super+0x24a/0x540 drivers/usb/gadget/legacy/inode.c:1987 [ 43.417154] [] mount_single+0xf1/0x160 fs/super.c:1146 [ 43.417154] [] gadgetfs_mount+0x2c/0x40 drivers/usb/gadget/legacy/inode.c:2013 [ 43.417154] [] mount_fs+0x97/0x2e0 fs/super.c:1177 [ 43.417154] [] vfs_kern_mount.part.24+0x67/0x2f0 fs/namespace.c:954 [ 43.417154] [< inline >] vfs_kern_mount fs/namespace.c:2433 [ 43.417154] [< inline >] do_new_mount fs/namespace.c:2436 [ 43.417154] [] do_mount+0x418/0x2da0 fs/namespace.c:2758 [ 43.417154] [< inline >] SYSC_mount fs/namespace.c:2974 [ 43.417154] [] SyS_mount+0xab/0x120 fs/namespace.c:2951 [ 43.417154] [] entry_SYSCALL_64_fastpath+0x1f/0xc2 Freed: PID = 0 (stack is not available) Memory state around the buggy address: 88003c47e100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 88003c47e180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >88003c47e200: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc ^ 88003c47e280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 88003c47e300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ==
net/gadget: slab-out-of-bounds write in dev_config
Hi! I've got the following error report while running the syzkaller fuzzer. ep0_write() doesn't check the length, so a user can cause an out-of-bounds with both size and data controlled. There's a comment which says "IN DATA+STATUS caller makes len <= wLength". While I'm not exactly sure what that means, the length seems to be passed unmodified directly from dev_config(). This doesn't seem to be a critical security issue since gadgetfs can't be mounted from a user namespace. On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). == BUG: KASAN: slab-out-of-bounds in dev_config+0x86f/0x1190 at addr 88003c47e160 Write of size 65537 by task syz-executor0/6356 CPU: 3 PID: 6356 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #19 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 88003c107ad8 81f96aba 3dc11ef0 110007820eee ed0007820ee6 88003dc11f00 41b58ab3 8598b4c8 81f96828 813fb4a0 88003b6eadc0 88003c107738 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x292/0x398 lib/dump_stack.c:51 [] kasan_object_err+0x1c/0x70 mm/kasan/report.c:159 [< inline >] print_address_description mm/kasan/report.c:197 [] kasan_report_error+0x1f0/0x4e0 mm/kasan/report.c:286 [] kasan_report+0x35/0x40 mm/kasan/report.c:306 [< inline >] check_memory_region_inline mm/kasan/kasan.c:308 [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315 [] kasan_check_write+0x14/0x20 mm/kasan/kasan.c:326 [< inline >] copy_from_user arch/x86/include/asm/uaccess.h:689 [< inline >] ep0_write drivers/usb/gadget/legacy/inode.c:1135 [] dev_config+0x86f/0x1190 drivers/usb/gadget/legacy/inode.c:1759 [] __vfs_write+0x5d5/0x760 fs/read_write.c:510 [] vfs_write+0x170/0x4e0 fs/read_write.c:560 [< inline >] SYSC_write fs/read_write.c:607 [] SyS_write+0xfb/0x230 fs/read_write.c:599 [] entry_SYSCALL_64_fastpath+0x1f/0xc2 Object at 88003c47e038, in cache kmalloc-1024 size: 1024 Allocated: PID = 4565 [ 43.417154] [] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57 [ 43.417154] [] save_stack+0x43/0xd0 mm/kasan/kasan.c:495 [ 43.417154] [< inline >] set_track mm/kasan/kasan.c:507 [ 43.417154] [] kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:598 [ 43.417154] [] kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2735 [ 43.417154] [< inline >] kmalloc include/linux/slab.h:490 [ 43.417154] [< inline >] kzalloc include/linux/slab.h:636 [ 43.417154] [< inline >] dev_new drivers/usb/gadget/legacy/inode.c:170 [ 43.417154] [] gadgetfs_fill_super+0x24a/0x540 drivers/usb/gadget/legacy/inode.c:1987 [ 43.417154] [] mount_single+0xf1/0x160 fs/super.c:1146 [ 43.417154] [] gadgetfs_mount+0x2c/0x40 drivers/usb/gadget/legacy/inode.c:2013 [ 43.417154] [] mount_fs+0x97/0x2e0 fs/super.c:1177 [ 43.417154] [] vfs_kern_mount.part.24+0x67/0x2f0 fs/namespace.c:954 [ 43.417154] [< inline >] vfs_kern_mount fs/namespace.c:2433 [ 43.417154] [< inline >] do_new_mount fs/namespace.c:2436 [ 43.417154] [] do_mount+0x418/0x2da0 fs/namespace.c:2758 [ 43.417154] [< inline >] SYSC_mount fs/namespace.c:2974 [ 43.417154] [] SyS_mount+0xab/0x120 fs/namespace.c:2951 [ 43.417154] [] entry_SYSCALL_64_fastpath+0x1f/0xc2 Freed: PID = 0 (stack is not available) Memory state around the buggy address: 88003c47e100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 88003c47e180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >88003c47e200: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc ^ 88003c47e280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 88003c47e300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ==