Commit 1deb9d341d475ff84262e927d6c0e36fecb9942e

Authored by Jiri Kosina
1 parent b52b506161

HID: debug: fix RCU preemption issue

Commit 2353f2bea ("HID: protect hid_debug_list") introduced mutex
locking around debug_list access to prevent SMP races when debugfs
nodes are being operated upon by multiple userspace processess.

mutex is not a proper synchronization primitive though, as the hid-debug
callbacks are being called from atomic contexts.

We also have to be careful about disabling IRQs when taking the lock
to prevent deadlock against IRQ handlers.

Benjamin reports this has also been reported in RH bugzilla as bug #958935.

 ===============================
 [ INFO: suspicious RCU usage. ]
 3.9.0+ #94 Not tainted
 -------------------------------
 include/linux/rcupdate.h:476 Illegal context switch in RCU read-side critical section!

 other info that might help us debug this:

 rcu_scheduler_active = 1, debug_locks = 0
 4 locks held by Xorg/5502:
  #0:  (&evdev->mutex){+.+...}, at: [<ffffffff81512c3d>] evdev_write+0x6d/0x160
  #1:  (&(&dev->event_lock)->rlock#2){-.-...}, at: [<ffffffff8150dd9b>] input_inject_event+0x5b/0x230
  #2:  (rcu_read_lock){.+.+..}, at: [<ffffffff8150dd82>] input_inject_event+0x42/0x230
  #3:  (&(&usbhid->lock)->rlock){-.....}, at: [<ffffffff81565289>] usb_hidinput_input_event+0x89/0x120

 stack backtrace:
 CPU: 0 PID: 5502 Comm: Xorg Not tainted 3.9.0+ #94
 Hardware name: Dell Inc. OptiPlex 390/0M5DCD, BIOS A09 07/24/2012
  0000000000000001 ffff8800689c7c38 ffffffff816f249f ffff8800689c7c68
  ffffffff810acb1d 0000000000000000 ffffffff81a03ac7 000000000000019d
  0000000000000000 ffff8800689c7c90 ffffffff8107cda7 0000000000000000
 Call Trace:
  [<ffffffff816f249f>] dump_stack+0x19/0x1b
  [<ffffffff810acb1d>] lockdep_rcu_suspicious+0xfd/0x130
  [<ffffffff8107cda7>] __might_sleep+0xc7/0x230
  [<ffffffff816f7770>] mutex_lock_nested+0x40/0x3a0
  [<ffffffff81312ac4>] ? vsnprintf+0x354/0x640
  [<ffffffff81553cc4>] hid_debug_event+0x34/0x100
  [<ffffffff81554197>] hid_dump_input+0x67/0xa0
  [<ffffffff81556430>] hid_set_field+0x50/0x120
  [<ffffffff8156529a>] usb_hidinput_input_event+0x9a/0x120
  [<ffffffff8150d89e>] input_handle_event+0x8e/0x530
  [<ffffffff8150df10>] input_inject_event+0x1d0/0x230
  [<ffffffff8150dd82>] ? input_inject_event+0x42/0x230
  [<ffffffff81512cae>] evdev_write+0xde/0x160
  [<ffffffff81185038>] vfs_write+0xc8/0x1f0
  [<ffffffff81185535>] SyS_write+0x55/0xa0
  [<ffffffff81704482>] system_call_fastpath+0x16/0x1b
 BUG: sleeping function called from invalid context at kernel/mutex.c:413
 in_atomic(): 1, irqs_disabled(): 1, pid: 5502, name: Xorg
 INFO: lockdep is turned off.
 irq event stamp: 1098574
 hardirqs last  enabled at (1098573): [<ffffffff816fb53f>] _raw_spin_unlock_irqrestore+0x3f/0x70
 hardirqs last disabled at (1098574): [<ffffffff816faaf5>] _raw_spin_lock_irqsave+0x25/0xa0
 softirqs last  enabled at (1098306): [<ffffffff8104971f>] __do_softirq+0x18f/0x3c0
 softirqs last disabled at (1097867): [<ffffffff81049ad5>] irq_exit+0xa5/0xb0
 CPU: 0 PID: 5502 Comm: Xorg Not tainted 3.9.0+ #94
 Hardware name: Dell Inc. OptiPlex 390/0M5DCD, BIOS A09 07/24/2012
  ffffffff81a03ac7 ffff8800689c7c68 ffffffff816f249f ffff8800689c7c90
  ffffffff8107ce60 0000000000000000 ffff8800689c7fd8 ffff88006a62c800
  ffff8800689c7d10 ffffffff816f7770 ffff8800689c7d00 ffffffff81312ac4
 Call Trace:
  [<ffffffff816f249f>] dump_stack+0x19/0x1b
  [<ffffffff8107ce60>] __might_sleep+0x180/0x230
  [<ffffffff816f7770>] mutex_lock_nested+0x40/0x3a0
  [<ffffffff81312ac4>] ? vsnprintf+0x354/0x640
  [<ffffffff81553cc4>] hid_debug_event+0x34/0x100
  [<ffffffff81554197>] hid_dump_input+0x67/0xa0
  [<ffffffff81556430>] hid_set_field+0x50/0x120
  [<ffffffff8156529a>] usb_hidinput_input_event+0x9a/0x120
  [<ffffffff8150d89e>] input_handle_event+0x8e/0x530
  [<ffffffff8150df10>] input_inject_event+0x1d0/0x230
  [<ffffffff8150dd82>] ? input_inject_event+0x42/0x230
  [<ffffffff81512cae>] evdev_write+0xde/0x160
  [<ffffffff81185038>] vfs_write+0xc8/0x1f0
  [<ffffffff81185535>] SyS_write+0x55/0xa0
  [<ffffffff81704482>] system_call_fastpath+0x16/0x1b

Reported-by: majianpeng <majianpeng@gmail.com>
Reported-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>

Showing 3 changed files with 11 additions and 8 deletions Side-by-side Diff

drivers/hid/hid-core.c
... ... @@ -2342,7 +2342,7 @@
2342 2342  
2343 2343 init_waitqueue_head(&hdev->debug_wait);
2344 2344 INIT_LIST_HEAD(&hdev->debug_list);
2345   - mutex_init(&hdev->debug_list_lock);
  2345 + spin_lock_init(&hdev->debug_list_lock);
2346 2346 sema_init(&hdev->driver_lock, 1);
2347 2347 sema_init(&hdev->driver_input_lock, 1);
2348 2348  
drivers/hid/hid-debug.c
... ... @@ -579,15 +579,16 @@
579 579 {
580 580 int i;
581 581 struct hid_debug_list *list;
  582 + unsigned long flags;
582 583  
583   - mutex_lock(&hdev->debug_list_lock);
  584 + spin_lock_irqsave(&hdev->debug_list_lock, flags);
584 585 list_for_each_entry(list, &hdev->debug_list, node) {
585 586 for (i = 0; i < strlen(buf); i++)
586 587 list->hid_debug_buf[(list->tail + i) % HID_DEBUG_BUFSIZE] =
587 588 buf[i];
588 589 list->tail = (list->tail + i) % HID_DEBUG_BUFSIZE;
589 590 }
590   - mutex_unlock(&hdev->debug_list_lock);
  591 + spin_unlock_irqrestore(&hdev->debug_list_lock, flags);
591 592  
592 593 wake_up_interruptible(&hdev->debug_wait);
593 594 }
... ... @@ -977,6 +978,7 @@
977 978 {
978 979 int err = 0;
979 980 struct hid_debug_list *list;
  981 + unsigned long flags;
980 982  
981 983 if (!(list = kzalloc(sizeof(struct hid_debug_list), GFP_KERNEL))) {
982 984 err = -ENOMEM;
983 985  
... ... @@ -992,9 +994,9 @@
992 994 file->private_data = list;
993 995 mutex_init(&list->read_mutex);
994 996  
995   - mutex_lock(&list->hdev->debug_list_lock);
  997 + spin_lock_irqsave(&list->hdev->debug_list_lock, flags);
996 998 list_add_tail(&list->node, &list->hdev->debug_list);
997   - mutex_unlock(&list->hdev->debug_list_lock);
  999 + spin_unlock_irqrestore(&list->hdev->debug_list_lock, flags);
998 1000  
999 1001 out:
1000 1002 return err;
1001 1003  
1002 1004  
... ... @@ -1088,10 +1090,11 @@
1088 1090 static int hid_debug_events_release(struct inode *inode, struct file *file)
1089 1091 {
1090 1092 struct hid_debug_list *list = file->private_data;
  1093 + unsigned long flags;
1091 1094  
1092   - mutex_lock(&list->hdev->debug_list_lock);
  1095 + spin_lock_irqsave(&list->hdev->debug_list_lock, flags);
1093 1096 list_del(&list->node);
1094   - mutex_unlock(&list->hdev->debug_list_lock);
  1097 + spin_unlock_irqrestore(&list->hdev->debug_list_lock, flags);
1095 1098 kfree(list->hid_debug_buf);
1096 1099 kfree(list);
1097 1100  
... ... @@ -515,7 +515,7 @@
515 515 struct dentry *debug_rdesc;
516 516 struct dentry *debug_events;
517 517 struct list_head debug_list;
518   - struct mutex debug_list_lock;
  518 + spinlock_t debug_list_lock;
519 519 wait_queue_head_t debug_wait;
520 520 };
521 521