2020-05-06 04:09:55

by Kyungtae Kim

[permalink] [raw]
Subject: [PATCH] USB: gadget: fix illegal array access in binding with UDC

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 <[email protected]>
Reported-and-tested-by: Kyungtae Kim <[email protected]>

---
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 32b637e3e1fa..6a9aa4413d64 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;
--
2.17.1


2020-05-09 08:00:31

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] USB: gadget: fix illegal array access in binding with UDC

Kyungtae Kim <[email protected]> writes:

> 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 <[email protected]>
> Reported-and-tested-by: Kyungtae Kim <[email protected]>
>
> ---
> 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 32b637e3e1fa..6a9aa4413d64 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,

patch is line-wrapped, please fix and resend.

--
balbi


Attachments:
signature.asc (847.00 B)