From ba18311dff7933ccb9c41bbbb1ad3d70840069b5 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 12 Jan 2012 17:42:22 +0800 Subject: HID: usbhid: fix dead lock between open and disconect There is no reason to hold hiddev->existancelock before calling usb_deregister_dev, so move it out of the lock. The patch fixes the lockdep warning below. [ 5733.386271] ====================================================== [ 5733.386274] [ INFO: possible circular locking dependency detected ] [ 5733.386278] 3.2.0-custom-next-20120111+ #1 Not tainted [ 5733.386281] ------------------------------------------------------- [ 5733.386284] khubd/186 is trying to acquire lock: [ 5733.386288] (minor_rwsem){++++.+}, at: [] usb_deregister_dev+0x37/0x9e [usbcore] [ 5733.386311] [ 5733.386312] but task is already holding lock: [ 5733.386315] (&hiddev->existancelock){+.+...}, at: [] hiddev_disconnect+0x26/0x87 [usbhid] [ 5733.386328] [ 5733.386329] which lock already depends on the new lock. [ 5733.386330] [ 5733.386333] [ 5733.386334] the existing dependency chain (in reverse order) is: [ 5733.386336] [ 5733.386337] -> #1 (&hiddev->existancelock){+.+...}: [ 5733.386346] [] lock_acquire+0xcb/0x10e [ 5733.386357] [] __mutex_lock_common+0x60/0x465 [ 5733.386366] [] mutex_lock_nested+0x36/0x3b [ 5733.386371] [] hiddev_open+0x113/0x193 [usbhid] [ 5733.386378] [] usb_open+0x66/0xc2 [usbcore] [ 5733.386390] [] chrdev_open+0x12b/0x154 [ 5733.386402] [] __dentry_open.isra.16+0x20b/0x355 [ 5733.386408] [] nameidata_to_filp+0x43/0x4a [ 5733.386413] [] do_last+0x536/0x570 [ 5733.386419] [] path_openat+0xce/0x301 [ 5733.386423] [] do_filp_open+0x33/0x81 [ 5733.386427] [] do_sys_open+0x6a/0xfc [ 5733.386431] [] sys_open+0x1c/0x1e [ 5733.386434] [] system_call_fastpath+0x16/0x1b [ 5733.386441] [ 5733.386441] -> #0 (minor_rwsem){++++.+}: [ 5733.386448] [] __lock_acquire+0xa80/0xd74 [ 5733.386454] [] lock_acquire+0xcb/0x10e [ 5733.386458] [] down_write+0x44/0x77 [ 5733.386464] [] usb_deregister_dev+0x37/0x9e [usbcore] [ 5733.386475] [] hiddev_disconnect+0x3c/0x87 [usbhid] [ 5733.386483] [] hid_disconnect+0x3f/0x54 [ 5733.386491] [] hid_device_remove+0x4e/0x7a [ 5733.386496] [] __device_release_driver+0x81/0xcd [ 5733.386502] [] device_release_driver+0x20/0x2d [ 5733.386507] [] bus_remove_device+0x114/0x128 [ 5733.386512] [] device_del+0x131/0x183 [ 5733.386519] [] hid_destroy_device+0x1e/0x3d [ 5733.386525] [] usbhid_disconnect+0x36/0x42 [usbhid] [ 5733.386530] [] usb_unbind_interface+0x57/0x11f [usbcore] [ 5733.386542] [] __device_release_driver+0x81/0xcd [ 5733.386547] [] device_release_driver+0x20/0x2d [ 5733.386552] [] bus_remove_device+0x114/0x128 [ 5733.386557] [] device_del+0x131/0x183 [ 5733.386562] [] usb_disable_device+0xa8/0x1d8 [usbcore] [ 5733.386573] [] usb_disconnect+0xab/0x11f [usbcore] [ 5733.386583] [] hub_thread+0x73b/0x1157 [usbcore] [ 5733.386593] [] kthread+0x95/0x9d [ 5733.386601] [] kernel_thread_helper+0x4/0x10 [ 5733.386607] [ 5733.386608] other info that might help us debug this: [ 5733.386609] [ 5733.386612] Possible unsafe locking scenario: [ 5733.386613] [ 5733.386615] CPU0 CPU1 [ 5733.386618] ---- ---- [ 5733.386620] lock(&hiddev->existancelock); [ 5733.386625] lock(minor_rwsem); [ 5733.386630] lock(&hiddev->existancelock); [ 5733.386635] lock(minor_rwsem); [ 5733.386639] [ 5733.386640] *** DEADLOCK *** [ 5733.386641] [ 5733.386644] 6 locks held by khubd/186: [ 5733.386646] #0: (&__lockdep_no_validate__){......}, at: [] hub_thread+0x14a/0x1157 [usbcore] [ 5733.386661] #1: (&__lockdep_no_validate__){......}, at: [] usb_disconnect+0x50/0x11f [usbcore] [ 5733.386677] #2: (hcd->bandwidth_mutex){+.+.+.}, at: [] usb_disconnect+0xa1/0x11f [usbcore] [ 5733.386693] #3: (&__lockdep_no_validate__){......}, at: [] device_release_driver+0x18/0x2d [ 5733.386704] #4: (&__lockdep_no_validate__){......}, at: [] device_release_driver+0x18/0x2d [ 5733.386714] #5: (&hiddev->existancelock){+.+...}, at: [] hiddev_disconnect+0x26/0x87 [usbhid] [ 5733.386727] [ 5733.386727] stack backtrace: [ 5733.386731] Pid: 186, comm: khubd Not tainted 3.2.0-custom-next-20120111+ #1 [ 5733.386734] Call Trace: [ 5733.386741] [] ? up+0x34/0x3b [ 5733.386747] [] print_circular_bug+0x1f8/0x209 [ 5733.386752] [] __lock_acquire+0xa80/0xd74 [ 5733.386756] [] ? trace_hardirqs_on_caller+0x15d/0x1a3 [ 5733.386763] [] ? vprintk+0x3f4/0x419 [ 5733.386774] [] ? usb_deregister_dev+0x37/0x9e [usbcore] [ 5733.386779] [] lock_acquire+0xcb/0x10e [ 5733.386789] [] ? usb_deregister_dev+0x37/0x9e [usbcore] [ 5733.386797] [] down_write+0x44/0x77 [ 5733.386807] [] ? usb_deregister_dev+0x37/0x9e [usbcore] [ 5733.386818] [] usb_deregister_dev+0x37/0x9e [usbcore] [ 5733.386825] [] hiddev_disconnect+0x3c/0x87 [usbhid] [ 5733.386830] [] hid_disconnect+0x3f/0x54 [ 5733.386834] [] hid_device_remove+0x4e/0x7a [ 5733.386839] [] __device_release_driver+0x81/0xcd [ 5733.386844] [] device_release_driver+0x20/0x2d [ 5733.386848] [] bus_remove_device+0x114/0x128 [ 5733.386854] [] device_del+0x131/0x183 [ 5733.386859] [] hid_destroy_device+0x1e/0x3d [ 5733.386865] [] usbhid_disconnect+0x36/0x42 [usbhid] [ 5733.386876] [] usb_unbind_interface+0x57/0x11f [usbcore] [ 5733.386882] [] __device_release_driver+0x81/0xcd [ 5733.386886] [] device_release_driver+0x20/0x2d [ 5733.386890] [] bus_remove_device+0x114/0x128 [ 5733.386895] [] device_del+0x131/0x183 [ 5733.386905] [] usb_disable_device+0xa8/0x1d8 [usbcore] [ 5733.386916] [] usb_disconnect+0xab/0x11f [usbcore] [ 5733.386921] [] ? __mutex_unlock_slowpath+0x130/0x141 [ 5733.386929] [] hub_thread+0x73b/0x1157 [usbcore] [ 5733.386935] [] ? finish_task_switch+0x78/0x150 [ 5733.386941] [] ? __init_waitqueue_head+0x4c/0x4c [ 5733.386950] [] ? usb_remote_wakeup+0x56/0x56 [usbcore] [ 5733.386955] [] kthread+0x95/0x9d [ 5733.386961] [] kernel_thread_helper+0x4/0x10 [ 5733.386966] [] ? retint_restore_args+0x13/0x13 [ 5733.386970] [] ? __init_kthread_worker+0x55/0x55 [ 5733.386974] [] ? gs_change+0x13/0x13 Signed-off-by: Ming Lei Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hiddev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c index 7c297d3..b1ec0e2 100644 --- a/drivers/hid/usbhid/hiddev.c +++ b/drivers/hid/usbhid/hiddev.c @@ -922,11 +922,11 @@ void hiddev_disconnect(struct hid_device *hid) struct hiddev *hiddev = hid->hiddev; struct usbhid_device *usbhid = hid->driver_data; + usb_deregister_dev(usbhid->intf, &hiddev_class); + mutex_lock(&hiddev->existancelock); hiddev->exist = 0; - usb_deregister_dev(usbhid->intf, &hiddev_class); - if (hiddev->open) { mutex_unlock(&hiddev->existancelock); usbhid_close(hiddev->hid); -- cgit v1.1