HID: fix a lockup regression when using force feedback on a PID device

Commit 8006479c9b75fb6594a7b746af3d7f1fbb68f18f introduced a spinlock in
input_dev->event_lock, which is locked when handling input events.
However, the hid-pidff driver sleeps when handling events as it waits for
reports being sent to the device before changing the report contents
again.
This causes a system lockup when trying to use force feedback with a PID
device, a regression introduced in 2.6.24 and 2.6.23.15.

Fix it by extracting the raw report data from struct hid_report
immediately when hid_submit_report() is called, therefore allowing
drivers to change the contents of struct hid_report immediately without
affecting the already-queued transfer.

In hid-pidff, re-add the removed usbhid_wait_io() to
pidff_erase_effect() instead, to prevent a full report queue from causing
the submission to fail, thus not freeing up device memory.
pidff_erase_effect() is not called while dev->event_lock is held.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>
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 07840df56c63e7f055298631bb73a990d0b4beb4..1ae047cd4fa16e0d70036cdfd1603b1861157aa2 100644 (file)
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -232,13 +232,16 @@ static void hid_irq_in(struct urb *urb)
 static int hid_submit_out(struct hid_device *hid)
 {
        struct hid_report *report;
+       char *raw_report;
        struct usbhid_device *usbhid = hid->driver_data;
 
-       report = usbhid->out[usbhid->outtail];
+       report = usbhid->out[usbhid->outtail].report;
+       raw_report = usbhid->out[usbhid->outtail].raw_report;
 
-       hid_output_report(report, usbhid->outbuf);
        usbhid->urbout->transfer_buffer_length = ((report->size - 1) >> 3) + 1 + (report->id > 0);
        usbhid->urbout->dev = hid_to_usb_dev(hid);
+       memcpy(usbhid->outbuf, raw_report, usbhid->urbout->transfer_buffer_length);
+       kfree(raw_report);
 
        dbg_hid("submitting out urb\n");
 
@@ -254,17 +257,20 @@ static int hid_submit_ctrl(struct hid_device *hid)
 {
        struct hid_report *report;
        unsigned char dir;
+       char *raw_report;
        int len;
        struct usbhid_device *usbhid = hid->driver_data;
 
        report = usbhid->ctrl[usbhid->ctrltail].report;
+       raw_report = usbhid->ctrl[usbhid->ctrltail].raw_report;
        dir = usbhid->ctrl[usbhid->ctrltail].dir;
 
        len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
        if (dir == USB_DIR_OUT) {
-               hid_output_report(report, usbhid->ctrlbuf);
                usbhid->urbctrl->pipe = usb_sndctrlpipe(hid_to_usb_dev(hid), 0);
                usbhid->urbctrl->transfer_buffer_length = len;
+               memcpy(usbhid->ctrlbuf, raw_report, len);
+               kfree(raw_report);
        } else {
                int maxpacket, padlen;
 
@@ -401,6 +407,7 @@ void usbhid_submit_report(struct hid_device *hid, struct hid_report *report, uns
        int head;
        unsigned long flags;
        struct usbhid_device *usbhid = hid->driver_data;
+       int len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
 
        if ((hid->quirks & HID_QUIRK_NOGET) && dir == USB_DIR_IN)
                return;
@@ -415,7 +422,14 @@ void usbhid_submit_report(struct hid_device *hid, struct hid_report *report, uns
                        return;
                }
 
-               usbhid->out[usbhid->outhead] = report;
+               usbhid->out[usbhid->outhead].raw_report = kmalloc(len, GFP_ATOMIC);
+               if (!usbhid->out[usbhid->outhead].raw_report) {
+                       spin_unlock_irqrestore(&usbhid->outlock, flags);
+                       warn("output queueing failed");
+                       return;
+               }
+               hid_output_report(report, usbhid->out[usbhid->outhead].raw_report);
+               usbhid->out[usbhid->outhead].report = report;
                usbhid->outhead = head;
 
                if (!test_and_set_bit(HID_OUT_RUNNING, &usbhid->iofl))
@@ -434,6 +448,15 @@ void usbhid_submit_report(struct hid_device *hid, struct hid_report *report, uns
                return;
        }
 
+       if (dir == USB_DIR_OUT) {
+               usbhid->ctrl[usbhid->ctrlhead].raw_report = kmalloc(len, GFP_ATOMIC);
+               if (!usbhid->ctrl[usbhid->ctrlhead].raw_report) {
+                       spin_unlock_irqrestore(&usbhid->ctrllock, flags);
+                       warn("control queueing failed");
+                       return;
+               }
+               hid_output_report(report, usbhid->ctrl[usbhid->ctrlhead].raw_report);
+       }
        usbhid->ctrl[usbhid->ctrlhead].report = report;
        usbhid->ctrl[usbhid->ctrlhead].dir = dir;
        usbhid->ctrlhead = head;

diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 011326178c0670c5b5e680a47d741661101a45aa..484e3eec2f88be2b740536fdae06cdc2e60dbc65 100644 (file)
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -397,7 +397,6 @@ static void pidff_set_condition_report(struct pidff_device *pidff,
                          effect->u.condition[i].left_saturation);
                pidff_set(&pidff->set_condition[PID_DEAD_BAND],
                          effect->u.condition[i].deadband);
-               usbhid_wait_io(pidff->hid);
                usbhid_submit_report(pidff->hid, pidff->reports[PID_SET_CONDITION],
                                  USB_DIR_OUT);
        }
@@ -512,7 +511,6 @@ static void pidff_playback_pid(struct pidff_device *pidff, int pid_id, int n)
                pidff->effect_operation[PID_LOOP_COUNT].value[0] = n;
        }
 
-       usbhid_wait_io(pidff->hid);
        usbhid_submit_report(pidff->hid, pidff->reports[PID_EFFECT_OPERATION],
                          USB_DIR_OUT);
 }
@@ -548,6 +546,9 @@ static int pidff_erase_effect(struct input_dev *dev, int effect_id)
        int pid_id = pidff->pid_id[effect_id];
 
        debug("starting to erase %d/%d", effect_id, pidff->pid_id[effect_id]);
+       /* Wait for the queue to clear. We do not want a full fifo to
+          prevent the effect removal. */
+       usbhid_wait_io(pidff->hid);
        pidff_playback_pid(pidff, pid_id, 0);
        pidff_erase_pid(pidff, pid_id);
 

diff --git a/drivers/hid/usbhid/usbhid.h b/drivers/hid/usbhid/usbhid.h
index b47f991867e9d096680bde9b7aa8a528f4e8cdb7..abedb13c623e419b12b8c3cc02cef4a8a977f520 100644 (file)
--- a/drivers/hid/usbhid/usbhid.h
+++ b/drivers/hid/usbhid/usbhid.h
@@ -67,7 +67,7 @@ struct usbhid_device {
        spinlock_t ctrllock;                                            /* Control fifo spinlock */
 
        struct urb *urbout;                                             /* Output URB */
-       struct hid_report *out[HID_CONTROL_FIFO_SIZE];                  /* Output pipe fifo */
+       struct hid_output_fifo out[HID_CONTROL_FIFO_SIZE];              /* Output pipe fifo */
        unsigned char outhead, outtail;                                 /* Output pipe fifo head & tail */
        char *outbuf;                                                   /* Output buffer */
        dma_addr_t outbuf_dma;                                          /* Output buffer dma */

diff --git a/include/linux/hid.h b/include/linux/hid.h
index dcdef0bb4bbaa4a4f63af05a9ffcd63003cb8a2e..f13bca2dd53b806303577cb298562a3a426b79b4 100644 (file)
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -388,6 +388,12 @@ struct hid_report_enum {
 struct hid_control_fifo {
        unsigned char dir;
        struct hid_report *report;
+       char *raw_report;
+};
+
+struct hid_output_fifo {
+       struct hid_report *report;
+       char *raw_report;
 };
 
 #define HID_CLAIMED_INPUT      1