[PATCH] usb: hub: Fix unbalanced reference count and memory leak

From: Viresh Kumar
Date: Wed Jul 27 2016 - 18:24:54 EST


If the hub gets disconnected while the core is still activating it, this
can result in leaking memory of few USB structures.

This will happen if we have done a kref_get() from hub_activate() and
scheduled a delayed work item for HUB_INIT2/3. Now if hub_disconnect()
gets called before the delayed work expires, then we will cancel the
work from hub_quiesce(), but wouldn't do a kref_put(). And so the
unbalance.

kmemleak reports this as (with the commit e50293ef9775 backported to
3.10 kernel with other changes, though the same is true for mainline as
well):

unreferenced object 0xffffffc08af5b800 (size 1024):
comm "khubd", pid 73, jiffies 4295051211 (age 6482.350s)
hex dump (first 32 bytes):
30 68 f3 8c c0 ff ff ff 00 a0 b2 2e c0 ff ff ff 0h..............
01 00 00 00 00 00 00 00 00 94 7d 40 c0 ff ff ff ..........}@....
backtrace:
[<ffffffc0003079ec>] create_object+0x148/0x2a0
[<ffffffc000cc150c>] kmemleak_alloc+0x80/0xbc
[<ffffffc000303a7c>] kmem_cache_alloc_trace+0x120/0x1ac
[<ffffffc0006fa610>] hub_probe+0x120/0xb84
[<ffffffc000702b20>] usb_probe_interface+0x1ec/0x298
[<ffffffc0005d50cc>] driver_probe_device+0x160/0x374
[<ffffffc0005d5308>] __device_attach+0x28/0x4c
[<ffffffc0005d3164>] bus_for_each_drv+0x78/0xac
[<ffffffc0005d4ee0>] device_attach+0x6c/0x9c
[<ffffffc0005d42b8>] bus_probe_device+0x28/0xa0
[<ffffffc0005d23a4>] device_add+0x324/0x604
[<ffffffc000700fcc>] usb_set_configuration+0x660/0x6cc
[<ffffffc00070a350>] generic_probe+0x44/0x84
[<ffffffc000702914>] usb_probe_device+0x54/0x74
[<ffffffc0005d50cc>] driver_probe_device+0x160/0x374
[<ffffffc0005d5308>] __device_attach+0x28/0x4c

Fix this by putting the reference in hub_quiesce() if we canceled a
pending work.

CC: <stable@xxxxxxxxxxxxxxx> #4.4+
Fixes: e50293ef9775 ("USB: fix invalid memory access in hub_activate()")
Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
---
Greg,

This is tested over 3.10 with backported patches only, sorry didn't had
a mainline setup to test this out. :(

drivers/usb/core/hub.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee13517676f..3173693fa8e3 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1315,7 +1315,8 @@ static void hub_quiesce(struct usb_hub *hub, enum hub_quiescing_type type)
struct usb_device *hdev = hub->hdev;
int i;

- cancel_delayed_work_sync(&hub->init_work);
+ if (cancel_delayed_work_sync(&hub->init_work))
+ kref_put(&hub->kref, hub_release);

/* hub_wq and related activity won't re-trigger */
hub->quiescing = 1;
--
2.7.4