diff mbox series

[SRU,X/B/D/E/F] USB: gadget: fix illegal array access in binding with UDC

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

Commit Message

Thadeu Lima de Souza Cascardo May 22, 2020, 10:37 p.m. UTC
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>
---
 drivers/usb/gadget/configfs.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Stefan Bader May 25, 2020, 1:37 p.m. UTC | #1
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;
>
Kleber Souza May 29, 2020, 6:39 a.m. UTC | #2
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 mbox series

Patch

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;