| Message ID | 20200522223752.514945-2-cascardo@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,X/B/D/E/F] USB: gadget: fix illegal array access in binding with UDC | expand |
On 23.05.20 00:37, Thadeu Lima de Souza Cascardo wrote: > From: Kyungtae Kim <kt0755@gmail.com> > > FuzzUSB (a variant of syzkaller) found an illegal array access > using an incorrect index while binding a gadget with UDC. > > Reference: https://www.spinics.net/lists/linux-usb/msg194331.html > > This bug occurs when a size variable used for a buffer > is misused to access its strcpy-ed buffer. > Given a buffer along with its size variable (taken from user input), > from which, a new buffer is created using kstrdup(). > Due to the original buffer containing 0 value in the middle, > the size of the kstrdup-ed buffer becomes smaller than that of the original. > So accessing the kstrdup-ed buffer with the same size variable > triggers memory access violation. > > The fix makes sure no zero value in the buffer, > by comparing the strlen() of the orignal buffer with the size variable, > so that the access to the kstrdup-ed buffer is safe. > > BUG: KASAN: slab-out-of-bounds in gadget_dev_desc_UDC_store+0x1ba/0x200 > drivers/usb/gadget/configfs.c:266 > Read of size 1 at addr ffff88806a55dd7e by task syz-executor.0/17208 > > CPU: 2 PID: 17208 Comm: syz-executor.0 Not tainted 5.6.8 #1 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0xce/0x128 lib/dump_stack.c:118 > print_address_description.constprop.4+0x21/0x3c0 mm/kasan/report.c:374 > __kasan_report+0x131/0x1b0 mm/kasan/report.c:506 > kasan_report+0x12/0x20 mm/kasan/common.c:641 > __asan_report_load1_noabort+0x14/0x20 mm/kasan/generic_report.c:132 > gadget_dev_desc_UDC_store+0x1ba/0x200 drivers/usb/gadget/configfs.c:266 > flush_write_buffer fs/configfs/file.c:251 [inline] > configfs_write_file+0x2f1/0x4c0 fs/configfs/file.c:283 > __vfs_write+0x85/0x110 fs/read_write.c:494 > vfs_write+0x1cd/0x510 fs/read_write.c:558 > ksys_write+0x18a/0x220 fs/read_write.c:611 > __do_sys_write fs/read_write.c:623 [inline] > __se_sys_write fs/read_write.c:620 [inline] > __x64_sys_write+0x73/0xb0 fs/read_write.c:620 > do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > Signed-off-by: Kyungtae Kim <kt0755@gmail.com> > Reported-and-tested-by: Kyungtae Kim <kt0755@gmail.com> > Cc: Felipe Balbi <balbi@kernel.org> > Cc: stable <stable@vger.kernel.org> > Link: https://lore.kernel.org/r/20200510054326.GA19198@pizza01 > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (cherry picked from commit 15753588bcd4bbffae1cca33c8ced5722477fe1f) > CVE-2020-13143 > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > drivers/usb/gadget/configfs.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c > index ab9ac48a751a..a7709d126b29 100644 > --- a/drivers/usb/gadget/configfs.c > +++ b/drivers/usb/gadget/configfs.c > @@ -260,6 +260,9 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item, > char *name; > int ret; > > + if (strlen(page) < len) > + return -EOVERFLOW; > + > name = kstrdup(page, GFP_KERNEL); > if (!name) > return -ENOMEM; >
On 2020-05-23 00:37, Thadeu Lima de Souza Cascardo wrote: > From: Kyungtae Kim <kt0755@gmail.com> > > FuzzUSB (a variant of syzkaller) found an illegal array access > using an incorrect index while binding a gadget with UDC. > > Reference: https://www.spinics.net/lists/linux-usb/msg194331.html > > This bug occurs when a size variable used for a buffer > is misused to access its strcpy-ed buffer. > Given a buffer along with its size variable (taken from user input), > from which, a new buffer is created using kstrdup(). > Due to the original buffer containing 0 value in the middle, > the size of the kstrdup-ed buffer becomes smaller than that of the original. > So accessing the kstrdup-ed buffer with the same size variable > triggers memory access violation. > > The fix makes sure no zero value in the buffer, > by comparing the strlen() of the orignal buffer with the size variable, > so that the access to the kstrdup-ed buffer is safe. > > BUG: KASAN: slab-out-of-bounds in gadget_dev_desc_UDC_store+0x1ba/0x200 > drivers/usb/gadget/configfs.c:266 > Read of size 1 at addr ffff88806a55dd7e by task syz-executor.0/17208 > > CPU: 2 PID: 17208 Comm: syz-executor.0 Not tainted 5.6.8 #1 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0xce/0x128 lib/dump_stack.c:118 > print_address_description.constprop.4+0x21/0x3c0 mm/kasan/report.c:374 > __kasan_report+0x131/0x1b0 mm/kasan/report.c:506 > kasan_report+0x12/0x20 mm/kasan/common.c:641 > __asan_report_load1_noabort+0x14/0x20 mm/kasan/generic_report.c:132 > gadget_dev_desc_UDC_store+0x1ba/0x200 drivers/usb/gadget/configfs.c:266 > flush_write_buffer fs/configfs/file.c:251 [inline] > configfs_write_file+0x2f1/0x4c0 fs/configfs/file.c:283 > __vfs_write+0x85/0x110 fs/read_write.c:494 > vfs_write+0x1cd/0x510 fs/read_write.c:558 > ksys_write+0x18a/0x220 fs/read_write.c:611 > __do_sys_write fs/read_write.c:623 [inline] > __se_sys_write fs/read_write.c:620 [inline] > __x64_sys_write+0x73/0xb0 fs/read_write.c:620 > do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > Signed-off-by: Kyungtae Kim <kt0755@gmail.com> > Reported-and-tested-by: Kyungtae Kim <kt0755@gmail.com> > Cc: Felipe Balbi <balbi@kernel.org> > Cc: stable <stable@vger.kernel.org> > Link: https://lore.kernel.org/r/20200510054326.GA19198@pizza01 > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (cherry picked from commit 15753588bcd4bbffae1cca33c8ced5722477fe1f) > CVE-2020-13143 > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> Clean cherry-pick, good test results: Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > drivers/usb/gadget/configfs.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c > index ab9ac48a751a..a7709d126b29 100644 > --- a/drivers/usb/gadget/configfs.c > +++ b/drivers/usb/gadget/configfs.c > @@ -260,6 +260,9 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item, > char *name; > int ret; > > + if (strlen(page) < len) > + return -EOVERFLOW; > + > name = kstrdup(page, GFP_KERNEL); > if (!name) > return -ENOMEM; >
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index ab9ac48a751a..a7709d126b29 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -260,6 +260,9 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item, char *name; int ret; + if (strlen(page) < len) + return -EOVERFLOW; + name = kstrdup(page, GFP_KERNEL); if (!name) return -ENOMEM;