mirror of
https://github.com/Divested-Mobile/DivestOS-Build.git
synced 2024-10-01 01:35:54 -04:00
265 lines
11 KiB
Diff
265 lines
11 KiB
Diff
From f16443a034c7aa359ddf6f0f9bc40d01ca31faea Mon Sep 17 00:00:00 2001
|
|
From: Alan Stern <stern@rowland.harvard.edu>
|
|
Date: Tue, 13 Jun 2017 15:23:42 -0400
|
|
Subject: [PATCH] USB: gadgetfs, dummy-hcd, net2280: fix locking for callbacks
|
|
|
|
Using the syzkaller kernel fuzzer, Andrey Konovalov generated the
|
|
following error in gadgetfs:
|
|
|
|
> BUG: KASAN: use-after-free in __lock_acquire+0x3069/0x3690
|
|
> kernel/locking/lockdep.c:3246
|
|
> Read of size 8 at addr ffff88003a2bdaf8 by task kworker/3:1/903
|
|
>
|
|
> CPU: 3 PID: 903 Comm: kworker/3:1 Not tainted 4.12.0-rc4+ #35
|
|
> 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 [inline]
|
|
> 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 [inline]
|
|
> kasan_report+0x230/0x340 mm/kasan/report.c:408
|
|
> __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:429
|
|
> __lock_acquire+0x3069/0x3690 kernel/locking/lockdep.c:3246
|
|
> lock_acquire+0x22d/0x560 kernel/locking/lockdep.c:3855
|
|
> __raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
|
|
> _raw_spin_lock+0x2f/0x40 kernel/locking/spinlock.c:151
|
|
> spin_lock include/linux/spinlock.h:299 [inline]
|
|
> gadgetfs_suspend+0x89/0x130 drivers/usb/gadget/legacy/inode.c:1682
|
|
> set_link_state+0x88e/0xae0 drivers/usb/gadget/udc/dummy_hcd.c:455
|
|
> dummy_hub_control+0xd7e/0x1fb0 drivers/usb/gadget/udc/dummy_hcd.c:2074
|
|
> rh_call_control drivers/usb/core/hcd.c:689 [inline]
|
|
> rh_urb_enqueue drivers/usb/core/hcd.c:846 [inline]
|
|
> usb_hcd_submit_urb+0x92f/0x20b0 drivers/usb/core/hcd.c:1650
|
|
> usb_submit_urb+0x8b2/0x12c0 drivers/usb/core/urb.c:542
|
|
> usb_start_wait_urb+0x148/0x5b0 drivers/usb/core/message.c:56
|
|
> usb_internal_control_msg drivers/usb/core/message.c:100 [inline]
|
|
> usb_control_msg+0x341/0x4d0 drivers/usb/core/message.c:151
|
|
> usb_clear_port_feature+0x74/0xa0 drivers/usb/core/hub.c:412
|
|
> hub_port_disable+0x123/0x510 drivers/usb/core/hub.c:4177
|
|
> hub_port_init+0x1ed/0x2940 drivers/usb/core/hub.c:4648
|
|
> hub_port_connect drivers/usb/core/hub.c:4826 [inline]
|
|
> hub_port_connect_change drivers/usb/core/hub.c:4999 [inline]
|
|
> port_event drivers/usb/core/hub.c:5105 [inline]
|
|
> hub_event+0x1ae1/0x3d40 drivers/usb/core/hub.c:5185
|
|
> process_one_work+0xc08/0x1bd0 kernel/workqueue.c:2097
|
|
> process_scheduled_works kernel/workqueue.c:2157 [inline]
|
|
> worker_thread+0xb2b/0x1860 kernel/workqueue.c:2233
|
|
> kthread+0x363/0x440 kernel/kthread.c:231
|
|
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:424
|
|
>
|
|
> Allocated by task 9958:
|
|
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
|
|
> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
|
|
> set_track mm/kasan/kasan.c:525 [inline]
|
|
> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:617
|
|
> kmem_cache_alloc_trace+0x87/0x280 mm/slub.c:2745
|
|
> kmalloc include/linux/slab.h:492 [inline]
|
|
> kzalloc include/linux/slab.h:665 [inline]
|
|
> dev_new drivers/usb/gadget/legacy/inode.c:170 [inline]
|
|
> gadgetfs_fill_super+0x24f/0x540 drivers/usb/gadget/legacy/inode.c:1993
|
|
> mount_single+0xf6/0x160 fs/super.c:1192
|
|
> gadgetfs_mount+0x31/0x40 drivers/usb/gadget/legacy/inode.c:2019
|
|
> mount_fs+0x9c/0x2d0 fs/super.c:1223
|
|
> vfs_kern_mount.part.25+0xcb/0x490 fs/namespace.c:976
|
|
> vfs_kern_mount fs/namespace.c:2509 [inline]
|
|
> do_new_mount fs/namespace.c:2512 [inline]
|
|
> do_mount+0x41b/0x2d90 fs/namespace.c:2834
|
|
> SYSC_mount fs/namespace.c:3050 [inline]
|
|
> SyS_mount+0xb0/0x120 fs/namespace.c:3027
|
|
> entry_SYSCALL_64_fastpath+0x1f/0xbe
|
|
>
|
|
> Freed by task 9960:
|
|
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
|
|
> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
|
|
> set_track mm/kasan/kasan.c:525 [inline]
|
|
> kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:590
|
|
> slab_free_hook mm/slub.c:1357 [inline]
|
|
> slab_free_freelist_hook mm/slub.c:1379 [inline]
|
|
> slab_free mm/slub.c:2961 [inline]
|
|
> kfree+0xed/0x2b0 mm/slub.c:3882
|
|
> put_dev+0x124/0x160 drivers/usb/gadget/legacy/inode.c:163
|
|
> gadgetfs_kill_sb+0x33/0x60 drivers/usb/gadget/legacy/inode.c:2027
|
|
> deactivate_locked_super+0x8d/0xd0 fs/super.c:309
|
|
> deactivate_super+0x21e/0x310 fs/super.c:340
|
|
> cleanup_mnt+0xb7/0x150 fs/namespace.c:1112
|
|
> __cleanup_mnt+0x1b/0x20 fs/namespace.c:1119
|
|
> task_work_run+0x1a0/0x280 kernel/task_work.c:116
|
|
> exit_task_work include/linux/task_work.h:21 [inline]
|
|
> do_exit+0x18a8/0x2820 kernel/exit.c:878
|
|
> do_group_exit+0x14e/0x420 kernel/exit.c:982
|
|
> get_signal+0x784/0x1780 kernel/signal.c:2318
|
|
> do_signal+0xd7/0x2130 arch/x86/kernel/signal.c:808
|
|
> exit_to_usermode_loop+0x1ac/0x240 arch/x86/entry/common.c:157
|
|
> prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
|
|
> syscall_return_slowpath+0x3ba/0x410 arch/x86/entry/common.c:263
|
|
> entry_SYSCALL_64_fastpath+0xbc/0xbe
|
|
>
|
|
> The buggy address belongs to the object at ffff88003a2bdae0
|
|
> which belongs to the cache kmalloc-1024 of size 1024
|
|
> The buggy address is located 24 bytes inside of
|
|
> 1024-byte region [ffff88003a2bdae0, ffff88003a2bdee0)
|
|
> The buggy address belongs to the page:
|
|
> page:ffffea0000e8ae00 count:1 mapcount:0 mapping: (null)
|
|
> index:0x0 compound_mapcount: 0
|
|
> flags: 0x100000000008100(slab|head)
|
|
> raw: 0100000000008100 0000000000000000 0000000000000000 0000000100170017
|
|
> raw: ffffea0000ed3020 ffffea0000f5f820 ffff88003e80efc0 0000000000000000
|
|
> page dumped because: kasan: bad access detected
|
|
>
|
|
> Memory state around the buggy address:
|
|
> ffff88003a2bd980: fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
|
|
> ffff88003a2bda00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
|
|
> >ffff88003a2bda80: fc fc fc fc fc fc fc fc fc fc fc fc fb fb fb fb
|
|
> ^
|
|
> ffff88003a2bdb00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
|
|
> ffff88003a2bdb80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
|
|
> ==================================================================
|
|
|
|
What this means is that the gadgetfs_suspend() routine was trying to
|
|
access dev->lock after it had been deallocated. The root cause is a
|
|
race in the dummy_hcd driver; the dummy_udc_stop() routine can race
|
|
with the rest of the driver because it contains no locking. And even
|
|
when proper locking is added, it can still race with the
|
|
set_link_state() function because that function incorrectly drops the
|
|
private spinlock before invoking any gadget driver callbacks.
|
|
|
|
The result of this race, as seen above, is that set_link_state() can
|
|
invoke a callback in gadgetfs even after gadgetfs has been unbound
|
|
from dummy_hcd's UDC and its private data structures have been
|
|
deallocated.
|
|
|
|
include/linux/usb/gadget.h documents that the ->reset, ->disconnect,
|
|
->suspend, and ->resume callbacks may be invoked in interrupt context.
|
|
In general this is necessary, to prevent races with gadget driver
|
|
removal. This patch fixes dummy_hcd to retain the spinlock across
|
|
these calls, and it adds a spinlock acquisition to dummy_udc_stop() to
|
|
prevent the race.
|
|
|
|
The net2280 driver makes the same mistake of dropping the private
|
|
spinlock for its ->disconnect and ->reset callback invocations. The
|
|
patch fixes it too.
|
|
|
|
Lastly, since gadgetfs_suspend() may be invoked in interrupt context,
|
|
it cannot assume that interrupts are enabled when it runs. It must
|
|
use spin_lock_irqsave() instead of spin_lock_irq(). The patch fixes
|
|
that bug as well.
|
|
|
|
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
|
|
Reported-and-tested-by: Andrey Konovalov <andreyknvl@google.com>
|
|
CC: <stable@vger.kernel.org>
|
|
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
|
|
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
---
|
|
drivers/usb/gadget/legacy/inode.c | 5 +++--
|
|
drivers/usb/gadget/udc/dummy_hcd.c | 13 ++++---------
|
|
drivers/usb/gadget/udc/net2280.c | 9 +--------
|
|
3 files changed, 8 insertions(+), 19 deletions(-)
|
|
|
|
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
|
|
index 5ffd879f78868..684900fcfe24c 100644
|
|
--- a/drivers/usb/gadget/legacy/inode.c
|
|
+++ b/drivers/usb/gadget/legacy/inode.c
|
|
@@ -1679,9 +1679,10 @@ static void
|
|
gadgetfs_suspend (struct usb_gadget *gadget)
|
|
{
|
|
struct dev_data *dev = get_gadget_data (gadget);
|
|
+ unsigned long flags;
|
|
|
|
INFO (dev, "suspended from state %d\n", dev->state);
|
|
- spin_lock (&dev->lock);
|
|
+ spin_lock_irqsave(&dev->lock, flags);
|
|
switch (dev->state) {
|
|
case STATE_DEV_SETUP: // VERY odd... host died??
|
|
case STATE_DEV_CONNECTED:
|
|
@@ -1692,7 +1693,7 @@ gadgetfs_suspend (struct usb_gadget *gadget)
|
|
default:
|
|
break;
|
|
}
|
|
- spin_unlock (&dev->lock);
|
|
+ spin_unlock_irqrestore(&dev->lock, flags);
|
|
}
|
|
|
|
static struct usb_gadget_driver gadgetfs_driver = {
|
|
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
|
|
index ccabb51cb98da..7635fd7cc328c 100644
|
|
--- a/drivers/usb/gadget/udc/dummy_hcd.c
|
|
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
|
|
@@ -442,23 +442,16 @@ static void set_link_state(struct dummy_hcd *dum_hcd)
|
|
/* Report reset and disconnect events to the driver */
|
|
if (dum->driver && (disconnect || reset)) {
|
|
stop_activity(dum);
|
|
- spin_unlock(&dum->lock);
|
|
if (reset)
|
|
usb_gadget_udc_reset(&dum->gadget, dum->driver);
|
|
else
|
|
dum->driver->disconnect(&dum->gadget);
|
|
- spin_lock(&dum->lock);
|
|
}
|
|
} else if (dum_hcd->active != dum_hcd->old_active) {
|
|
- if (dum_hcd->old_active && dum->driver->suspend) {
|
|
- spin_unlock(&dum->lock);
|
|
+ if (dum_hcd->old_active && dum->driver->suspend)
|
|
dum->driver->suspend(&dum->gadget);
|
|
- spin_lock(&dum->lock);
|
|
- } else if (!dum_hcd->old_active && dum->driver->resume) {
|
|
- spin_unlock(&dum->lock);
|
|
+ else if (!dum_hcd->old_active && dum->driver->resume)
|
|
dum->driver->resume(&dum->gadget);
|
|
- spin_lock(&dum->lock);
|
|
- }
|
|
}
|
|
|
|
dum_hcd->old_status = dum_hcd->port_status;
|
|
@@ -983,7 +976,9 @@ static int dummy_udc_stop(struct usb_gadget *g)
|
|
struct dummy_hcd *dum_hcd = gadget_to_dummy_hcd(g);
|
|
struct dummy *dum = dum_hcd->dum;
|
|
|
|
+ spin_lock_irq(&dum->lock);
|
|
dum->driver = NULL;
|
|
+ spin_unlock_irq(&dum->lock);
|
|
|
|
return 0;
|
|
}
|
|
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
|
|
index 6cf07857eacaa..f2cbd7f8005e1 100644
|
|
--- a/drivers/usb/gadget/udc/net2280.c
|
|
+++ b/drivers/usb/gadget/udc/net2280.c
|
|
@@ -2470,11 +2470,8 @@ static void stop_activity(struct net2280 *dev, struct usb_gadget_driver *driver)
|
|
nuke(&dev->ep[i]);
|
|
|
|
/* report disconnect; the driver is already quiesced */
|
|
- if (driver) {
|
|
- spin_unlock(&dev->lock);
|
|
+ if (driver)
|
|
driver->disconnect(&dev->gadget);
|
|
- spin_lock(&dev->lock);
|
|
- }
|
|
|
|
usb_reinit(dev);
|
|
}
|
|
@@ -3348,8 +3345,6 @@ static void handle_stat0_irqs(struct net2280 *dev, u32 stat)
|
|
BIT(PCI_RETRY_ABORT_INTERRUPT))
|
|
|
|
static void handle_stat1_irqs(struct net2280 *dev, u32 stat)
|
|
-__releases(dev->lock)
|
|
-__acquires(dev->lock)
|
|
{
|
|
struct net2280_ep *ep;
|
|
u32 tmp, num, mask, scratch;
|
|
@@ -3390,14 +3385,12 @@ __acquires(dev->lock)
|
|
if (disconnect || reset) {
|
|
stop_activity(dev, dev->driver);
|
|
ep0_start(dev);
|
|
- spin_unlock(&dev->lock);
|
|
if (reset)
|
|
usb_gadget_udc_reset
|
|
(&dev->gadget, dev->driver);
|
|
else
|
|
(dev->driver->disconnect)
|
|
(&dev->gadget);
|
|
- spin_lock(&dev->lock);
|
|
return;
|
|
}
|
|
}
|