Re: usb/serial/visor: slab-out-of-bounds in palm_os_3_probe
From: Greg Kroah-Hartman
Date: Fri Sep 29 2017 - 04:38:06 EST
On Thu, Sep 28, 2017 at 07:57:46PM +0200, Andrey Konovalov wrote:
> Hi!
>
> I've got the following report while fuzzing the kernel with syzkaller.
>
> On commit dc972a67cc54585bd83ad811c4e9b6ab3dcd427e (4.14-rc2+).
>
> There's no check on the connection_info->num_ports value when
> iterating over ports.
>
> usb 1-1: Handspring Visor / Palm OS: port 162, is for unknown use
> usb 1-1: Handspring Visor / Palm OS: port 81, is for unknown use
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in palm_os_3_probe+0x4e4/0x570
> Read of size 1 at addr ffff8800686daa26 by task kworker/0:1/24
>
> CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 4.14.0-rc2-42748-gcd3da2fe6c25 #324
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> Call Trace:
> __dump_stack lib/dump_stack.c:16
> dump_stack+0x292/0x395 lib/dump_stack.c:52
> print_address_description+0x78/0x280 mm/kasan/report.c:252
> kasan_report_error mm/kasan/report.c:351
> kasan_report+0x23d/0x350 mm/kasan/report.c:409
> __asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427
> palm_os_3_probe+0x4e4/0x570 drivers/usb/serial/visor.c:348
> visor_probe+0x11c/0x1e0 drivers/usb/serial/visor.c:463
> usb_serial_probe+0x540/0x4090 drivers/usb/serial/usb-serial.c:903
> usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
> generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
> usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2538
> hub_port_connect drivers/usb/core/hub.c:4984
> hub_port_connect_change drivers/usb/core/hub.c:5090
> port_event drivers/usb/core/hub.c:5196
> hub_event_impl+0x1971/0x3760 drivers/usb/core/hub.c:5310
> hub_event+0x11b/0x3f0 drivers/usb/core/hub.c:5206
> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
> kthread+0x3a1/0x470 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> Allocated by task 24:
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> set_track mm/kasan/kasan.c:459
> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
> kmem_cache_alloc_trace+0x11e/0x2d0 mm/slub.c:2772
> kmalloc ./include/linux/slab.h:493
> palm_os_3_probe+0x7c/0x570 drivers/usb/serial/visor.c:325
> visor_probe+0x11c/0x1e0 drivers/usb/serial/visor.c:463
> usb_serial_probe+0x540/0x4090 drivers/usb/serial/usb-serial.c:903
> usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
> generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
> usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2538
> hub_port_connect drivers/usb/core/hub.c:4984
> hub_port_connect_change drivers/usb/core/hub.c:5090
> port_event drivers/usb/core/hub.c:5196
> hub_event_impl+0x1971/0x3760 drivers/usb/core/hub.c:5310
> hub_event+0x11b/0x3f0 drivers/usb/core/hub.c:5206
> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
> kthread+0x3a1/0x470 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> Freed by task 2772:
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> set_track mm/kasan/kasan.c:459
> kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:524
> slab_free_hook mm/slub.c:1390
> slab_free_freelist_hook mm/slub.c:1412
> slab_free mm/slub.c:2988
> kfree+0xf6/0x2f0 mm/slub.c:3919
> signalfd_release+0x3c/0x50 fs/signalfd.c:56
> __fput+0x33e/0x800 fs/file_table.c:210
> ____fput+0x1a/0x20 fs/file_table.c:244
> task_work_run+0x1af/0x280 kernel/task_work.c:112
> tracehook_notify_resume ./include/linux/tracehook.h:191
> exit_to_usermode_loop+0x1d4/0x210 arch/x86/entry/common.c:162
> prepare_exit_to_usermode arch/x86/entry/common.c:197
> syscall_return_slowpath+0x3e2/0x4a0 arch/x86/entry/common.c:266
> entry_SYSCALL_64_fastpath+0xc0/0xc2 arch/x86/entry/entry_64.S:238
>
> The buggy address belongs to the object at ffff8800686daa20
> which belongs to the cache kmalloc-8 of size 8
> The buggy address is located 6 bytes inside of
> 8-byte region [ffff8800686daa20, ffff8800686daa28)
> The buggy address belongs to the page:
> page:ffffea0001a1b680 count:1 mapcount:0 mapping: (null) index:0x0
> flags: 0x100000000000100(slab)
> raw: 0100000000000100 0000000000000000 0000000000000000 0000000180aa00aa
> raw: ffffea00018f6e80 0000000500000005 ffff88006c403c80 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff8800686da900: fb fc fc fb fc fc fb fc fc fb fc fc fb fc fc fb
> ffff8800686da980: fc fc fb fc fc fb fc fc fb fc fc fb fc fc fb fc
> >ffff8800686daa00: fc fb fc fc 06 fc fc fb fc fc fb fc fc fb fc fc
> ^
> ffff8800686daa80: fb fc fc fb fc fc fb fc fc fb fc fc fb fc fc fb
> ffff8800686dab00: fc fc fb fc fc fb fc fc fb fc fc fb fc fc fb fc
> ==================================================================
Ah, nice catch, this bug is _old_, sorry about that.
The patch below should resolve this. It looks bigger than it really is,
as I'm just moving the error checking higher up in the function, and
loosing an indentation for when there is invalid data.
Can you let me know if this solves the issue?
thanks,
greg k-h
------------------------
diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c
index 9f3317a940ef..18c31bae0e10 100644
--- a/drivers/usb/serial/visor.c
+++ b/drivers/usb/serial/visor.c
@@ -338,47 +338,48 @@ static int palm_os_3_probe(struct usb_serial *serial,
goto exit;
}
- if (retval == sizeof(*connection_info)) {
- connection_info = (struct visor_connection_info *)
- transfer_buffer;
-
- num_ports = le16_to_cpu(connection_info->num_ports);
- for (i = 0; i < num_ports; ++i) {
- switch (
- connection_info->connections[i].port_function_id) {
- case VISOR_FUNCTION_GENERIC:
- string = "Generic";
- break;
- case VISOR_FUNCTION_DEBUGGER:
- string = "Debugger";
- break;
- case VISOR_FUNCTION_HOTSYNC:
- string = "HotSync";
- break;
- case VISOR_FUNCTION_CONSOLE:
- string = "Console";
- break;
- case VISOR_FUNCTION_REMOTE_FILE_SYS:
- string = "Remote File System";
- break;
- default:
- string = "unknown";
- break;
- }
- dev_info(dev, "%s: port %d, is for %s use\n",
- serial->type->description,
- connection_info->connections[i].port, string);
- }
+ if (retval != sizeof(*connection_info)) {
+ retval = -EINVAL;
+ goto exit;
}
- /*
- * Handle devices that report invalid stuff here.
- */
+
+ connection_info = (struct visor_connection_info *)transfer_buffer;
+
+ num_ports = le16_to_cpu(connection_info->num_ports);
+
+ /* Handle devices that report invalid stuff here. */
if (num_ports == 0 || num_ports > 2) {
dev_warn(dev, "%s: No valid connect info available\n",
serial->type->description);
num_ports = 2;
}
+ for (i = 0; i < num_ports; ++i) {
+ switch (
+ connection_info->connections[i].port_function_id) {
+ case VISOR_FUNCTION_GENERIC:
+ string = "Generic";
+ break;
+ case VISOR_FUNCTION_DEBUGGER:
+ string = "Debugger";
+ break;
+ case VISOR_FUNCTION_HOTSYNC:
+ string = "HotSync";
+ break;
+ case VISOR_FUNCTION_CONSOLE:
+ string = "Console";
+ break;
+ case VISOR_FUNCTION_REMOTE_FILE_SYS:
+ string = "Remote File System";
+ break;
+ default:
+ string = "unknown";
+ break;
+ }
+ dev_info(dev, "%s: port %d, is for %s use\n",
+ serial->type->description,
+ connection_info->connections[i].port, string);
+ }
dev_info(dev, "%s: Number of ports: %d\n", serial->type->description,
num_ports);