HID: fix oops during suspend of unbound HID devices
Usbhid structure is allocated on start invoked only from probe
of some driver. When there is no driver, the structure is null
and causes null-dereference oopses.
Fix it by allocating the structure on probe and disconnect of
the device itself. Also make sure we won't race between start
and resume or stop and suspend respectively.
References: http://bugzilla.kernel.org/show_bug.cgi?id=11827
Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Andreas Schwab <schwab@suse.de>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 42bdd83..3b1c489 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -20,6 +20,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/mm.h>
+#include <linux/mutex.h>
#include <linux/smp_lock.h>
#include <linux/spinlock.h>
#include <asm/unaligned.h>
@@ -776,21 +777,10 @@
struct usb_interface *intf = to_usb_interface(hid->dev.parent);
struct usb_host_interface *interface = intf->cur_altsetting;
struct usb_device *dev = interface_to_usbdev(intf);
- struct usbhid_device *usbhid;
+ struct usbhid_device *usbhid = hid->driver_data;
unsigned int n, insize = 0;
int ret;
- WARN_ON(hid->driver_data);
-
- usbhid = kzalloc(sizeof(struct usbhid_device), GFP_KERNEL);
- if (usbhid == NULL) {
- ret = -ENOMEM;
- goto err;
- }
-
- hid->driver_data = usbhid;
- usbhid->hid = hid;
-
usbhid->bufsize = HID_MIN_BUFFER_SIZE;
hid_find_max_report(hid, HID_INPUT_REPORT, &usbhid->bufsize);
hid_find_max_report(hid, HID_OUTPUT_REPORT, &usbhid->bufsize);
@@ -804,6 +794,7 @@
if (insize > HID_MAX_BUFFER_SIZE)
insize = HID_MAX_BUFFER_SIZE;
+ mutex_lock(&usbhid->setup);
if (hid_alloc_buffers(dev, hid)) {
ret = -ENOMEM;
goto fail;
@@ -888,6 +879,9 @@
usbhid_init_reports(hid);
hid_dump_device(hid);
+ set_bit(HID_STARTED, &usbhid->iofl);
+ mutex_unlock(&usbhid->setup);
+
return 0;
fail:
@@ -895,8 +889,7 @@
usb_free_urb(usbhid->urbout);
usb_free_urb(usbhid->urbctrl);
hid_free_buffers(dev, hid);
- kfree(usbhid);
-err:
+ mutex_unlock(&usbhid->setup);
return ret;
}
@@ -907,6 +900,8 @@
if (WARN_ON(!usbhid))
return;
+ mutex_lock(&usbhid->setup);
+ clear_bit(HID_STARTED, &usbhid->iofl);
spin_lock_irq(&usbhid->inlock); /* Sync with error handler */
set_bit(HID_DISCONNECTED, &usbhid->iofl);
spin_unlock_irq(&usbhid->inlock);
@@ -931,8 +926,7 @@
usb_free_urb(usbhid->urbout);
hid_free_buffers(hid_to_usb_dev(hid), hid);
- kfree(usbhid);
- hid->driver_data = NULL;
+ mutex_unlock(&usbhid->setup);
}
static struct hid_ll_driver usb_hid_driver = {
@@ -947,6 +941,7 @@
static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
{
struct usb_device *dev = interface_to_usbdev(intf);
+ struct usbhid_device *usbhid;
struct hid_device *hid;
size_t len;
int ret;
@@ -1000,14 +995,26 @@
if (usb_string(dev, dev->descriptor.iSerialNumber, hid->uniq, 64) <= 0)
hid->uniq[0] = 0;
+ usbhid = kzalloc(sizeof(*usbhid), GFP_KERNEL);
+ if (usbhid == NULL) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ hid->driver_data = usbhid;
+ usbhid->hid = hid;
+ mutex_init(&usbhid->setup); /* needed on suspend/resume */
+
ret = hid_add_device(hid);
if (ret) {
if (ret != -ENODEV)
dev_err(&intf->dev, "can't add hid device: %d\n", ret);
- goto err;
+ goto err_free;
}
return 0;
+err_free:
+ kfree(usbhid);
err:
hid_destroy_device(hid);
return ret;
@@ -1016,11 +1023,14 @@
static void hid_disconnect(struct usb_interface *intf)
{
struct hid_device *hid = usb_get_intfdata(intf);
+ struct usbhid_device *usbhid;
if (WARN_ON(!hid))
return;
+ usbhid = hid->driver_data;
hid_destroy_device(hid);
+ kfree(usbhid);
}
static int hid_suspend(struct usb_interface *intf, pm_message_t message)
@@ -1028,11 +1038,18 @@
struct hid_device *hid = usb_get_intfdata (intf);
struct usbhid_device *usbhid = hid->driver_data;
+ mutex_lock(&usbhid->setup);
+ if (!test_bit(HID_STARTED, &usbhid->iofl)) {
+ mutex_unlock(&usbhid->setup);
+ return 0;
+ }
+
spin_lock_irq(&usbhid->inlock); /* Sync with error handler */
set_bit(HID_SUSPENDED, &usbhid->iofl);
spin_unlock_irq(&usbhid->inlock);
del_timer(&usbhid->io_retry);
usb_kill_urb(usbhid->urbin);
+ mutex_unlock(&usbhid->setup);
dev_dbg(&intf->dev, "suspend\n");
return 0;
}
@@ -1043,9 +1060,16 @@
struct usbhid_device *usbhid = hid->driver_data;
int status;
+ mutex_lock(&usbhid->setup);
+ if (!test_bit(HID_STARTED, &usbhid->iofl)) {
+ mutex_unlock(&usbhid->setup);
+ return 0;
+ }
+
clear_bit(HID_SUSPENDED, &usbhid->iofl);
usbhid->retry_delay = 0;
status = hid_start_in(hid);
+ mutex_unlock(&usbhid->setup);
dev_dbg(&intf->dev, "resume status %d\n", status);
return status;
}