Skip to content

Commit

Permalink
HID: fix oops during suspend of unbound HID devices
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jiri Slaby authored and Jiri Kosina committed Oct 27, 2008
1 parent f8d56f1 commit 3d5afd3
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 17 deletions.
58 changes: 41 additions & 17 deletions drivers/hid/usbhid/hid-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -776,21 +777,10 @@ static int usbhid_start(struct hid_device *hid)
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);
Expand All @@ -804,6 +794,7 @@ static int usbhid_start(struct hid_device *hid)
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;
Expand Down Expand Up @@ -888,15 +879,17 @@ static int usbhid_start(struct hid_device *hid)
usbhid_init_reports(hid);
hid_dump_device(hid);

set_bit(HID_STARTED, &usbhid->iofl);
mutex_unlock(&usbhid->setup);

return 0;

fail:
usb_free_urb(usbhid->urbin);
usb_free_urb(usbhid->urbout);
usb_free_urb(usbhid->urbctrl);
hid_free_buffers(dev, hid);
kfree(usbhid);
err:
mutex_unlock(&usbhid->setup);
return ret;
}

Expand All @@ -907,6 +900,8 @@ static void usbhid_stop(struct hid_device *hid)
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);
Expand All @@ -931,8 +926,7 @@ static void usbhid_stop(struct hid_device *hid)
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 = {
Expand All @@ -947,6 +941,7 @@ static struct hid_ll_driver usb_hid_driver = {
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;
Expand Down Expand Up @@ -1000,14 +995,26 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
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;
Expand All @@ -1016,23 +1023,33 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
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)
{
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;
}
Expand All @@ -1043,9 +1060,16 @@ static int hid_resume(struct usb_interface *intf)
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;
}
Expand Down
2 changes: 2 additions & 0 deletions drivers/hid/usbhid/usbhid.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <linux/types.h>
#include <linux/slab.h>
#include <linux/list.h>
#include <linux/mutex.h>
#include <linux/timer.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
Expand Down Expand Up @@ -73,6 +74,7 @@ struct usbhid_device {
dma_addr_t outbuf_dma; /* Output buffer dma */
spinlock_t outlock; /* Output fifo spinlock */

struct mutex setup;
unsigned long iofl; /* I/O flags (CTRL_RUNNING, OUT_RUNNING) */
struct timer_list io_retry; /* Retry timer */
unsigned long stop_retry; /* Time to give up, in jiffies */
Expand Down
1 change: 1 addition & 0 deletions include/linux/hid.h
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ struct hid_output_fifo {
#define HID_SUSPENDED 5
#define HID_CLEAR_HALT 6
#define HID_DISCONNECTED 7
#define HID_STARTED 8

struct hid_input {
struct list_head list;
Expand Down

0 comments on commit 3d5afd3

Please sign in to comment.