HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

Commit Message

Colenbrander, Roderick May 31, 2016, 9:10 p.m. UTC

Hi,

The patch in this thread is part of input work I'm doing with my team and will hopefully be the first of many, but we are new to the linux-input project. We shared this patch first in April and again about 2 weeks ago in May, but we haven't received any feedback so far. Originally the discussion leading to this patch was partially on linux-input, but then privately with David and he suggested the direction the patch took. I think the resulting patch is clean and self explanatory. We have tested the patch on various devices without issues. For us this patch fixes an important problem and we would like to get it included in the upstream kernel.

Thanks,
Roderick Colenbrander
Gaikai Inc, a Sony Interactive Entertainment Company

Comments

Greg KH May 31, 2016, 9:59 p.m. UTC | #1

On Tue, May 31, 2016 at 09:10:36PM +0000, Colenbrander, Roelof wrote:
> Hi,
> 
> The patch in this thread is part of input work I'm doing with my team
> and will hopefully be the first of many, but we are new to the
> linux-input project. We shared this patch first in April and again
> about 2 weeks ago in May, but we haven't received any feedback so far.

It's been the middle of the merge window for the past 2 weeks, where no
new code is usually reviewed or handled at all.  Give the maintainers a
bit of time to catch up.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 16b6f11..99ec3ff 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -51,10 +51,26 @@  struct uhid_device {
        u32 report_id;
        u32 report_type;
        struct uhid_event report_buf;
+       struct work_struct worker;
 };

 static struct miscdevice uhid_misc;

+static void uhid_device_add_worker(struct work_struct *work)
+{
+       struct uhid_device *uhid = container_of(work, struct uhid_device, worker);
+       int ret;
+
+       ret = hid_add_device(uhid->hid);
+       if (ret) {
+               hid_err(uhid->hid, "Cannot register HID device: error %d\n", ret);
+
+               hid_destroy_device(uhid->hid);
+               uhid->hid = NULL;
+               uhid->running = false;
+       }
+}
+
 static void uhid_queue(struct uhid_device *uhid, struct uhid_event *ev)
 {
        __u8 newhead;
@@ -498,18 +514,14 @@  static int uhid_dev_create2(struct uhid_device *uhid,
        uhid->hid = hid;
        uhid->running = true;

-       ret = hid_add_device(hid);
-       if (ret) {
-               hid_err(hid, "Cannot register HID device\n");
-               goto err_hid;
-       }
+       /* Adding of a HID device is done through a worker, to allow HID drivers
+        * which use feature requests during .probe to work, without they would
+        * be blocked on devlock, which is held by uhid_char_write.
+        */
+       schedule_work(&uhid->worker);

        return 0;

-err_hid:
-       hid_destroy_device(hid);
-       uhid->hid = NULL;
-       uhid->running = false;
 err_free:
        kfree(uhid->rd_data);
        uhid->rd_data = NULL;
@@ -550,6 +562,8 @@  static int uhid_dev_destroy(struct uhid_device *uhid)
        uhid->running = false;
        wake_up_interruptible(&uhid->report_wait);

+       cancel_work_sync(&uhid->worker);
+
        hid_destroy_device(uhid->hid);
        kfree(uhid->rd_data);

@@ -612,6 +626,7 @@  static int uhid_char_open(struct inode *inode, struct file *file)
        init_waitqueue_head(&uhid->waitq);
        init_waitqueue_head(&uhid->report_wait);
        uhid->running = false;
+       INIT_WORK(&uhid->worker, uhid_device_add_worker);

        file->private_data = uhid;
        nonseekable_open(inode, file);