Message ID | 20161018120921.dhlo76bbkdhlm7ey@pd.tnic (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Oct 18, 2016 at 02:09:22PM +0200, Borislav Petkov wrote: > Adding more people to CC. > > I still see this after 4.8 is out. > > So PeterZ suggested something like this: > > --- > diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c > index fb4b185dea96..9112c3cecad7 100644 > --- a/drivers/input/mouse/psmouse-base.c > +++ b/drivers/input/mouse/psmouse-base.c > @@ -1420,14 +1420,16 @@ static void psmouse_disconnect(struct serio *serio) > psmouse_deactivate(parent); > } > > - if (psmouse->disconnect) > - psmouse->disconnect(psmouse); > - > if (parent && parent->pt_deactivate) > parent->pt_deactivate(parent); > > psmouse_set_state(psmouse, PSMOUSE_IGNORE); > > + mutex_unlock(&psmouse_mutex); > + if (psmouse->disconnect) > + psmouse->disconnect(psmouse); > + mutex_lock(&psmouse_mutex); > + > serio_close(serio); > serio_set_drvdata(serio, NULL); > input_unregister_device(psmouse->dev); > --- > > to fix the lock inversion but that might have the other problem of > being racy by maybe ->reconnect() accessing psmouse->private in > trackpoint_sync() outside of the psmouse_mutex lock and that won't be > nice. > > But someone more knowledgeable with this code should take a look and > suggest a proper fix. > > Thanks! > > (Leaving in the rest for reference). The splat still persists on up-to-date kernel. See below. That's for current -tip tree, but it's in mainline too. Nobody cares? ====================================================== WARNING: possible circular locking dependency detected 4.14.0-rc6-00555-g34aa400565bc #151 Tainted: G W ------------------------------------------------------ kworker/0:1/38 is trying to acquire lock: (kn->count#188){++++}, at: [<ffffffff8233ba60>] kernfs_remove_by_name_ns+0x40/0x80 but task is already holding lock: (psmouse_mutex){+.+.}, at: [<ffffffff82805b37>] psmouse_disconnect+0x67/0x160 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (psmouse_mutex){+.+.}: __mutex_lock+0x85/0x960 psmouse_attr_set_helper+0x2d/0x140 kernfs_fop_write+0x112/0x1a0 __vfs_write+0x23/0x130 vfs_write+0xc9/0x1d0 SyS_write+0x45/0xb0 entry_SYSCALL_64_fastpath+0x23/0xc2 -> #0 (kn->count#188){++++}: lock_acquire+0xc1/0x220 __kernfs_remove+0x248/0x2b0 kernfs_remove_by_name_ns+0x40/0x80 remove_files.isra.0+0x31/0x70 sysfs_remove_group+0x3d/0x80 trackpoint_disconnect+0x20/0x40 psmouse_disconnect+0x94/0x160 serio_disconnect_driver+0x2d/0x40 serio_driver_remove+0x11/0x20 device_release_driver_internal+0x160/0x230 serio_reconnect_subtree+0x4a/0xa0 serio_handle_event+0x1af/0x270 process_one_work+0x1ea/0x680 worker_thread+0x4d/0x3e0 kthread+0x145/0x180 ret_from_fork+0x2a/0x40 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(psmouse_mutex); lock(kn->count#188); lock(psmouse_mutex); lock(kn->count#188); *** DEADLOCK *** 6 locks held by kworker/0:1/38: #0: ((wq_completion)"events_long"){+.+.}, at: [<ffffffff8210f705>] process_one_work+0x165/0x680 #1: (serio_event_work){+.+.}, at: [<ffffffff8210f705>] process_one_work+0x165/0x680 #2: (serio_mutex){+.+.}, at: [<ffffffff827f3271>] serio_handle_event+0x21/0x270 #3: (&dev->mutex){....}, at: [<ffffffff826e9ac4>] device_release_driver_internal+0x34/0x230 #4: (&serio->drv_mutex){+.+.}, at: [<ffffffff827f235b>] serio_disconnect_driver+0x1b/0x40 #5: (psmouse_mutex){+.+.}, at: [<ffffffff82805b37>] psmouse_disconnect+0x67/0x160 stack backtrace: CPU: 0 PID: 38 Comm: kworker/0:1 Tainted: G W 4.14.0-rc6-00555-g34aa400565bc #151 Hardware name: LENOVO 20FXS09D2P/20FXS09D2P, BIOS R07ET71W (2.11 ) 09/26/2016 Workqueue: events_long serio_handle_event Call Trace: dump_stack+0x7c/0xbe print_circular_bug+0x202/0x380 ? lockdep_init_map_crosslock+0x20/0x20 check_prev_add+0x43f/0x7b0 ? __lock_acquire+0x133d/0x1550 __lock_acquire+0x133d/0x1550 lock_acquire+0xc1/0x220 ? kernfs_remove_by_name_ns+0x40/0x80 __kernfs_remove+0x248/0x2b0 ? kernfs_remove_by_name_ns+0x40/0x80 ? kernfs_name_hash+0x12/0x80 ? kernfs_find_ns+0x89/0x100 kernfs_remove_by_name_ns+0x40/0x80 remove_files.isra.0+0x31/0x70 sysfs_remove_group+0x3d/0x80 trackpoint_disconnect+0x20/0x40 psmouse_disconnect+0x94/0x160 serio_disconnect_driver+0x2d/0x40 serio_driver_remove+0x11/0x20 device_release_driver_internal+0x160/0x230 serio_reconnect_subtree+0x4a/0xa0 serio_handle_event+0x1af/0x270 process_one_work+0x1ea/0x680 worker_thread+0x4d/0x3e0 kthread+0x145/0x180 ? process_one_work+0x680/0x680 ? kthread_create_on_node+0x40/0x40 ret_from_fork+0x2a/0x40
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c index fb4b185dea96..9112c3cecad7 100644 --- a/drivers/input/mouse/psmouse-base.c +++ b/drivers/input/mouse/psmouse-base.c @@ -1420,14 +1420,16 @@ static void psmouse_disconnect(struct serio *serio) psmouse_deactivate(parent); } - if (psmouse->disconnect) - psmouse->disconnect(psmouse); - if (parent && parent->pt_deactivate) parent->pt_deactivate(parent); psmouse_set_state(psmouse, PSMOUSE_IGNORE); + mutex_unlock(&psmouse_mutex); + if (psmouse->disconnect) + psmouse->disconnect(psmouse); + mutex_lock(&psmouse_mutex); + serio_close(serio); serio_set_drvdata(serio, NULL); input_unregister_device(psmouse->dev);