From: Pete Zaitcev If a process is closing /dev/input/mice and an mouse disconnects simulta- neously, the system is likely to oops. This usually happens when someone hits F1 or logs out from X, and flips a KVM while the system is reacting. I reproduced the issue by running this: while true; do cat /dev/input/mice; done This way, it oopses on 2nd or 3rd disconnect reliably. With the patch, I can disconnect the mouse 20 times. Discussion One of the race scenarios is related to the list of handles. The cat calls mousedev_close -> mixdev_release, does list_for_each to walk for all handles for a given handler. Iterations are longish while it does input_close_device -> hidinput_close -> usbhid_close -> usb_kill_urb, which sleeps briefly. Into this gap goes khubd and does hid_disconnect -> hidinput_disconnect -> input_unregister_device. This corrupts the list of handles which cat process is walking. I was unable to devise a scheme to protect the stock h_list adequately, so I implemented a private list of mousedev instances, which can be protected correctly. Dmitry, please consider getting rid of the list of handles entirely. The other major user is drivers/char/keyboard.c. Other than that, the patch is straightforward. It adds a static mutex to guard common data structures. It has to be static because instances of mousedev share common structures, such as the mousedev_table[]. Signed-off-by: Pete Zaitcev Cc: Dmitry Torokhov Signed-off-by: Andrew Morton --- drivers/input/mousedev.c | 44 ++++++++++++++++++++++++++----------- 1 files changed, 32 insertions(+), 12 deletions(-) diff -puN drivers/input/mousedev.c~fix-locking-in-mousedev drivers/input/mousedev.c --- a/drivers/input/mousedev.c~fix-locking-in-mousedev +++ a/drivers/input/mousedev.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -64,6 +65,7 @@ struct mousedev { char name[16]; wait_queue_head_t wait; struct list_head list; + struct list_head h_node; struct input_handle handle; struct mousedev_hw_data packet; @@ -108,10 +110,13 @@ static unsigned char mousedev_imps_seq[] static unsigned char mousedev_imex_seq[] = { 0xf3, 200, 0xf3, 200, 0xf3, 80 }; static struct input_handler mousedev_handler; +static LIST_HEAD(mousedev_h_list); static struct mousedev *mousedev_table[MOUSEDEV_MINORS]; static struct mousedev mousedev_mix; +static DEFINE_MUTEX(mousedev_lock); + #define fx(i) (mousedev->old_x[(mousedev->pkt_count - (i)) & 03]) #define fy(i) (mousedev->old_y[(mousedev->pkt_count - (i)) & 03]) @@ -366,11 +371,9 @@ static void mousedev_free(struct mousede static void mixdev_release(void) { - struct input_handle *handle; - - list_for_each_entry(handle, &mousedev_handler.h_list, h_node) { - struct mousedev *mousedev = handle->private; + struct mousedev *mousedev; + list_for_each_entry(mousedev, &mousedev_h_list, h_node) { if (!mousedev->open) { if (mousedev->exist) input_close_device(&mousedev->handle); @@ -386,6 +389,7 @@ static int mousedev_release(struct inode mousedev_fasync(-1, file, 0); + mutex_lock(&mousedev_lock); list_del(&list->node); if (!--list->mousedev->open) { @@ -398,6 +402,7 @@ static int mousedev_release(struct inode mousedev_free(list->mousedev); } } + mutex_unlock(&mousedev_lock); kfree(list); return 0; @@ -406,7 +411,6 @@ static int mousedev_release(struct inode static int mousedev_open(struct inode * inode, struct file * file) { struct mousedev_list *list; - struct input_handle *handle; struct mousedev *mousedev; int i; @@ -417,11 +421,16 @@ static int mousedev_open(struct inode * #endif i = iminor(inode) - MOUSEDEV_MINOR_BASE; - if (i >= MOUSEDEV_MINORS || !mousedev_table[i]) + mutex_lock(&mousedev_lock); + if (i >= MOUSEDEV_MINORS || !mousedev_table[i]) { + mutex_unlock(&mousedev_lock); return -ENODEV; + } - if (!(list = kzalloc(sizeof(struct mousedev_list), GFP_KERNEL))) + if (!(list = kzalloc(sizeof(struct mousedev_list), GFP_KERNEL))) { + mutex_unlock(&mousedev_lock); return -ENOMEM; + } spin_lock_init(&list->packet_lock); list->pos_x = xres / 2; @@ -432,16 +441,16 @@ static int mousedev_open(struct inode * if (!list->mousedev->open++) { if (list->mousedev->minor == MOUSEDEV_MIX) { - list_for_each_entry(handle, &mousedev_handler.h_list, h_node) { - mousedev = handle->private; + list_for_each_entry(mousedev, &mousedev_h_list, h_node) { if (!mousedev->open && mousedev->exist) - input_open_device(handle); + input_open_device(&mousedev->handle); } } else if (!mousedev_mix.open && list->mousedev->exist) input_open_device(&list->mousedev->handle); } + mutex_unlock(&mousedev_lock); return 0; } @@ -604,7 +613,6 @@ static ssize_t mousedev_read(struct file return count; } -/* No kernel lock - fine */ static unsigned int mousedev_poll(struct file *file, poll_table *wait) { struct mousedev_list *list = file->private_data; @@ -631,14 +639,19 @@ static struct input_handle *mousedev_con struct class_device *cdev; int minor = 0; + mutex_lock(&mousedev_lock); + for (minor = 0; minor < MOUSEDEV_MINORS && mousedev_table[minor]; minor++); if (minor == MOUSEDEV_MINORS) { + mutex_unlock(&mousedev_lock); printk(KERN_ERR "mousedev: no more free mousedev devices\n"); return NULL; } - if (!(mousedev = kzalloc(sizeof(struct mousedev), GFP_KERNEL))) + if (!(mousedev = kzalloc(sizeof(struct mousedev), GFP_KERNEL))) { + mutex_unlock(&mousedev_lock); return NULL; + } INIT_LIST_HEAD(&mousedev->list); init_waitqueue_head(&mousedev->wait); @@ -664,6 +677,9 @@ static struct input_handle *mousedev_con sysfs_create_link(&input_class.subsys.kset.kobj, &cdev->kobj, mousedev->name); + list_add_tail(&mousedev->h_node, &mousedev_h_list); + + mutex_unlock(&mousedev_lock); return &mousedev->handle; } @@ -672,6 +688,9 @@ static void mousedev_disconnect(struct i struct mousedev *mousedev = handle->private; struct mousedev_list *list; + mutex_lock(&mousedev_lock); + list_del(&mousedev->h_node); + sysfs_remove_link(&input_class.subsys.kset.kobj, mousedev->name); class_device_destroy(&input_class, MKDEV(INPUT_MAJOR, MOUSEDEV_MINOR_BASE + mousedev->minor)); @@ -687,6 +706,7 @@ static void mousedev_disconnect(struct i input_close_device(handle); mousedev_free(mousedev); } + mutex_unlock(&mousedev_lock); } static const struct input_device_id mousedev_ids[] = { _