[1/1] HID: hidraw: need to grab minors_lock in hidraw_report_event

Commit Message

yonghua zheng Aug. 9, 2013, 8:45 a.m. UTC

Hi Jiri,

We met another kernel panic issue in hidraw_report_event while using
BT remote on our Android system, the panic log below shows a unhandle
kernel paging request at virtual address 001000f0, this can happen if
hidraw_report_event access a hidraw_list node which is already removed
by list_del() in hidraw_release.

One last thing list_del() does is set entry->next = LIST_POISON1;
where LIST_POISON1 is 00100100, and this should be able to explain why
panic address is 001000f0, hidraw_report_event try to access somewhere
in struct hidraw_list:

[ 1337.530941] Unable to handle kernel paging request at virtual
address 001000f0
[ 1337.538679] pgd = a0570000
[ 1337.541877] [001000f0] *pgd=00000000
[ 1337.545884] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[ 1337.551373] Modules linked in: mbt8787 fusion(O) gal3d amp_core(O)
[ 1337.557834] CPU: 0    Tainted: G           O  (3.4.50+ #1)
[ 1337.563512] PC is at hidraw_report_event+0x38/0x98
[ 1337.568462] LR is at hidraw_report_event+0x70/0x98
[ 1337.573412] pc : [<8035286c>]    lr : [<803528a4>]    psr: a0000013
[ 1337.573416] sp : a0eede30  ip : a0eede30  fp : a0eede54
[ 1337.575086] [amp_driver] [vpp isr] MsgQ full
[ 1337.589671] r10: a0b7e4c0  r9 : a035b800  r8 : a0bb002c
[ 1337.595068] r7 : a0b7e4e4  r6 : 00000001  r5 : 0000001f  r4 : 000ffef0
[ 1337.601809] r3 : a36db808  r2 : 00000020  r1 : 0000001f  r0 : a0bb002c
[ 1337.608553] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[ 1337.615923] Control: 10c5387d  Table: 2157004a  DAC: 00000015

So the fix is to grab the minors_lock to avoid list_del in the middle
of hidraw_report_event()

From 725ed6c402f36e34479739a8d6c68f78f2f96d31 Mon Sep 17 00:00:00 2001
From: Yonghua Zheng <[e-mail]>
Date: Fri, 9 Aug 2013 15:54:59 +0800
Subject: [PATCH 1/1] HID: hidraw: need to grab minors_lock in
 hidraw_report_event

It is unsafe to call list_for_each_entry to go through each hidraw_list
node without grabing the minors_lock, because the dev->list can be
modified if other process calls hidraw_release to list_del itself from
the same &dev->list, if this happens, it can result in kernel panic
due to unable handle kernel paging request at invalid virtual address

Signed-off-by: Yonghua Zheng <[e-mail]>
---
 drivers/hid/hidraw.c |    3 +++
 1 file changed, 3 insertions(+)


@@ -465,6 +466,7 @@ int hidraw_report_event(struct hid_device *hid, u8
*data, int len)
             continue;

         if (!(list->buffer[list->head].value = kmemdup(data, len,
GFP_ATOMIC))) {
+            mutex_unlock(&minors_lock);
             ret = -ENOMEM;
             break;
         }
@@ -472,6 +474,7 @@ int hidraw_report_event(struct hid_device *hid, u8
*data, int len)
         list->head = new_head;
         kill_fasync(&list->fasync, SIGIO, POLL_IN);
     }
+    mutex_unlock(&minors_lock);

     wake_up_interruptible(&dev->wait);
     return ret;

Patch

diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 6f1feb2..8b408c4 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -458,6 +458,7 @@  int hidraw_report_event(struct hid_device *hid, u8
*data, int len)
     struct hidraw_list *list;
     int ret = 0;

+    mutex_lock(&minors_lock);
     list_for_each_entry(list, &dev->list, node) {
         int new_head = (list->head + 1) & (HIDRAW_BUFFER_SIZE - 1);