Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid

Pull HID fixes from Jiri Kosina:

 - fix for race condition that could lead to NULL pointer dereferences
   or UAF during uhid device destruction (Jann Horn)

 - contact count handling regression fixes for Wacom devices (Jason
   Gerecke)

 - fix for handling unnumbered HID reports handling in Google Vivaldi
   driver (Dmitry Torokhov)

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid:
  HID: wacom: Avoid using stale array indicies to read contact count
  HID: wacom: Ignore the confidence flag when a touch is removed
  HID: wacom: Reset expected and received contact counts at the same time
  HID: uhid: Use READ_ONCE()/WRITE_ONCE() for ->running
  HID: uhid: Fix worker destroying device without any protection
  HID: vivaldi: Minor cleanups
  HID: vivaldi: fix handling devices not using numbered reports
  HID: Ignore battery for Elan touchscreen on HP Envy X360 15t-dr100

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 26cee452ec4417f9bbab398cd842a646660dc19a..85975031389b344e54957ed9711ddff85d8147e0 100644 (file)
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -400,6 +400,7 @@
 #define USB_DEVICE_ID_HP_X2            0x074d
 #define USB_DEVICE_ID_HP_X2_10_COVER   0x0755
 #define I2C_DEVICE_ID_HP_ENVY_X360_15  0x2d05
+#define I2C_DEVICE_ID_HP_ENVY_X360_15T_DR100   0x29CF
 #define I2C_DEVICE_ID_HP_SPECTRE_X360_15       0x2817
 #define USB_DEVICE_ID_ASUS_UX550VE_TOUCHSCREEN 0x2544
 #define USB_DEVICE_ID_ASUS_UX550_TOUCHSCREEN   0x2706

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 1ce75e8b49d5750a6faefde156e2a724185a2b32..112901d2d8d2a0db34e2df53e59c521fd0dae145 100644 (file)
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -330,6 +330,8 @@ static const struct hid_device_id hid_battery_quirks[] = {
          HID_BATTERY_QUIRK_IGNORE },
        { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_ENVY_X360_15),
          HID_BATTERY_QUIRK_IGNORE },
+       { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_ENVY_X360_15T_DR100),
+         HID_BATTERY_QUIRK_IGNORE },
        { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_SPECTRE_X360_15),
          HID_BATTERY_QUIRK_IGNORE },
        { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_SURFACE_GO_TOUCHSCREEN),

diff --git a/drivers/hid/hid-vivaldi.c b/drivers/hid/hid-vivaldi.c
index 72957a9f7117052fb7b7e9bdf0fd87d14018400e..efa6140915f4416182dfd9686c79ed3178f802be 100644 (file)
--- a/drivers/hid/hid-vivaldi.c
+++ b/drivers/hid/hid-vivaldi.c
@@ -6,16 +6,17 @@
  * Author: Sean O'Brien <seobrien@chromium.org>
  */
 
+#include <linux/device.h>
 #include <linux/hid.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/sysfs.h>
 
 #define MIN_FN_ROW_KEY 1
 #define MAX_FN_ROW_KEY 24
 #define HID_VD_FN_ROW_PHYSMAP 0x00000001
 #define HID_USAGE_FN_ROW_PHYSMAP (HID_UP_GOOGLEVENDOR | HID_VD_FN_ROW_PHYSMAP)
 
-static struct hid_driver hid_vivaldi;
-
 struct vivaldi_data {
        u32 function_row_physmap[MAX_FN_ROW_KEY - MIN_FN_ROW_KEY + 1];
        int max_function_row_key;
@@ -40,7 +41,7 @@ static ssize_t function_row_physmap_show(struct device *dev,
        return size;
 }
 
-DEVICE_ATTR_RO(function_row_physmap);
+static DEVICE_ATTR_RO(function_row_physmap);
 static struct attribute *sysfs_attrs[] = {
        &dev_attr_function_row_physmap.attr,
        NULL
@@ -74,10 +75,11 @@ static void vivaldi_feature_mapping(struct hid_device *hdev,
                                    struct hid_usage *usage)
 {
        struct vivaldi_data *drvdata = hid_get_drvdata(hdev);
+       struct hid_report *report = field->report;
        int fn_key;
        int ret;
        u32 report_len;
-       u8 *buf;
+       u8 *report_data, *buf;
 
        if (field->logical != HID_USAGE_FN_ROW_PHYSMAP ||
            (usage->hid & HID_USAGE_PAGE) != HID_UP_ORDINAL)
@@ -89,12 +91,24 @@ static void vivaldi_feature_mapping(struct hid_device *hdev,
        if (fn_key > drvdata->max_function_row_key)
                drvdata->max_function_row_key = fn_key;
 
-       buf = hid_alloc_report_buf(field->report, GFP_KERNEL);
-       if (!buf)
+       report_data = buf = hid_alloc_report_buf(report, GFP_KERNEL);
+       if (!report_data)
                return;
 
-       report_len = hid_report_len(field->report);
-       ret = hid_hw_raw_request(hdev, field->report->id, buf,
+       report_len = hid_report_len(report);
+       if (!report->id) {
+               /*
+                * hid_hw_raw_request() will stuff report ID (which will be 0)
+                * into the first byte of the buffer even for unnumbered
+                * reports, so we need to account for this to avoid getting
+                * -EOVERFLOW in return.
+                * Note that hid_alloc_report_buf() adds 7 bytes to the size
+                * so we can safely say that we have space for an extra byte.
+                */
+               report_len++;
+       }
+
+       ret = hid_hw_raw_request(hdev, report->id, report_data,
                                 report_len, HID_FEATURE_REPORT,
                                 HID_REQ_GET_REPORT);
        if (ret < 0) {
@@ -103,7 +117,16 @@ static void vivaldi_feature_mapping(struct hid_device *hdev,
                goto out;
        }
 
-       ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
+       if (!report->id) {
+               /*
+                * Undo the damage from hid_hw_raw_request() for unnumbered
+                * reports.
+                */
+               report_data++;
+               report_len--;
+       }
+
+       ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, report_data,
                                   report_len, 0);
        if (ret) {
                dev_warn(&hdev->dev, "failed to report feature %d\n",

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 8fe3efcb832715c4a978625387dc0bb0ea5b00a2..614adb510dbd985d9486e156e33a2511571c44df 100644 (file)
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -28,11 +28,22 @@
 
 struct uhid_device {
        struct mutex devlock;
+
+       /* This flag tracks whether the HID device is usable for commands from
+        * userspace. The flag is already set before hid_add_device(), which
+        * runs in workqueue context, to allow hid_add_device() to communicate
+        * with userspace.
+        * However, if hid_add_device() fails, the flag is cleared without
+        * holding devlock.
+        * We guarantee that if @running changes from true to false while you're
+        * holding @devlock, it's still fine to access @hid.
+        */
        bool running;
 
        __u8 *rd_data;
        uint rd_size;
 
+       /* When this is NULL, userspace may use UHID_CREATE/UHID_CREATE2. */
        struct hid_device *hid;
        struct uhid_event input_buf;
 
@@ -63,9 +74,18 @@ static void uhid_device_add_worker(struct work_struct *work)
        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;
+               /* We used to call hid_destroy_device() here, but that's really
+                * messy to get right because we have to coordinate with
+                * concurrent writes from userspace that might be in the middle
+                * of using uhid->hid.
+                * Just leave uhid->hid as-is for now, and clean it up when
+                * userspace tries to close or reinitialize the uhid instance.
+                *
+                * However, we do have to clear the ->running flag and do a
+                * wakeup to make sure userspace knows that the device is gone.
+                */
+               WRITE_ONCE(uhid->running, false);
+               wake_up_interruptible(&uhid->report_wait);
        }
 }
 
@@ -174,9 +194,9 @@ static int __uhid_report_queue_and_wait(struct uhid_device *uhid,
        spin_unlock_irqrestore(&uhid->qlock, flags);
 
        ret = wait_event_interruptible_timeout(uhid->report_wait,
-                               !uhid->report_running || !uhid->running,
+                               !uhid->report_running || !READ_ONCE(uhid->running),
                                5 * HZ);
-       if (!ret || !uhid->running || uhid->report_running)
+       if (!ret || !READ_ONCE(uhid->running) || uhid->report_running)
                ret = -EIO;
        else if (ret < 0)
                ret = -ERESTARTSYS;
@@ -217,7 +237,7 @@ static int uhid_hid_get_report(struct hid_device *hid, unsigned char rnum,
        struct uhid_event *ev;
        int ret;
 
-       if (!uhid->running)
+       if (!READ_ONCE(uhid->running))
                return -EIO;
 
        ev = kzalloc(sizeof(*ev), GFP_KERNEL);
@@ -259,7 +279,7 @@ static int uhid_hid_set_report(struct hid_device *hid, unsigned char rnum,
        struct uhid_event *ev;
        int ret;
 
-       if (!uhid->running || count > UHID_DATA_MAX)
+       if (!READ_ONCE(uhid->running) || count > UHID_DATA_MAX)
                return -EIO;
 
        ev = kzalloc(sizeof(*ev), GFP_KERNEL);
@@ -474,7 +494,7 @@ static int uhid_dev_create2(struct uhid_device *uhid,
        void *rd_data;
        int ret;
 
-       if (uhid->running)
+       if (uhid->hid)
                return -EALREADY;
 
        rd_size = ev->u.create2.rd_size;
@@ -556,15 +576,16 @@ static int uhid_dev_create(struct uhid_device *uhid,
 
 static int uhid_dev_destroy(struct uhid_device *uhid)
 {
-       if (!uhid->running)
+       if (!uhid->hid)
                return -EINVAL;
 
-       uhid->running = false;
+       WRITE_ONCE(uhid->running, false);
        wake_up_interruptible(&uhid->report_wait);
 
        cancel_work_sync(&uhid->worker);
 
        hid_destroy_device(uhid->hid);
+       uhid->hid = NULL;
        kfree(uhid->rd_data);
 
        return 0;
@@ -572,7 +593,7 @@ static int uhid_dev_destroy(struct uhid_device *uhid)
 
 static int uhid_dev_input(struct uhid_device *uhid, struct uhid_event *ev)
 {
-       if (!uhid->running)
+       if (!READ_ONCE(uhid->running))
                return -EINVAL;
 
        hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data,
@@ -583,7 +604,7 @@ static int uhid_dev_input(struct uhid_device *uhid, struct uhid_event *ev)
 
 static int uhid_dev_input2(struct uhid_device *uhid, struct uhid_event *ev)
 {
-       if (!uhid->running)
+       if (!READ_ONCE(uhid->running))
                return -EINVAL;
 
        hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input2.data,
@@ -595,7 +616,7 @@ static int uhid_dev_input2(struct uhid_device *uhid, struct uhid_event *ev)
 static int uhid_dev_get_report_reply(struct uhid_device *uhid,
                                     struct uhid_event *ev)
 {
-       if (!uhid->running)
+       if (!READ_ONCE(uhid->running))
                return -EINVAL;
 
        uhid_report_wake_up(uhid, ev->u.get_report_reply.id, ev);
@@ -605,7 +626,7 @@ static int uhid_dev_get_report_reply(struct uhid_device *uhid,
 static int uhid_dev_set_report_reply(struct uhid_device *uhid,
                                     struct uhid_event *ev)
 {
-       if (!uhid->running)
+       if (!READ_ONCE(uhid->running))
                return -EINVAL;
 
        uhid_report_wake_up(uhid, ev->u.set_report_reply.id, ev);

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 2a4cc39962e765b2eadf19490effa98fb3c7ae3c..a7176fc0635dd2b7aa0fd7977938f8feb9b6a2ec 100644 (file)
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2588,6 +2588,24 @@ static void wacom_wac_finger_slot(struct wacom_wac *wacom_wac,
        }
 }
 
+static bool wacom_wac_slot_is_active(struct input_dev *dev, int key)
+{
+       struct input_mt *mt = dev->mt;
+       struct input_mt_slot *s;
+
+       if (!mt)
+               return false;
+
+       for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
+               if (s->key == key &&
+                       input_mt_get_value(s, ABS_MT_TRACKING_ID) >= 0) {
+                       return true;
+               }
+       }
+
+       return false;
+}
+
 static void wacom_wac_finger_event(struct hid_device *hdev,
                struct hid_field *field, struct hid_usage *usage, __s32 value)
 {
@@ -2638,9 +2656,14 @@ static void wacom_wac_finger_event(struct hid_device *hdev,
        }
 
        if (usage->usage_index + 1 == field->report_count) {
-               if (equivalent_usage == wacom_wac->hid_data.last_slot_field &&
-                   wacom_wac->hid_data.confidence)
-                       wacom_wac_finger_slot(wacom_wac, wacom_wac->touch_input);
+               if (equivalent_usage == wacom_wac->hid_data.last_slot_field) {
+                       bool touch_removed = wacom_wac_slot_is_active(wacom_wac->touch_input,
+                               wacom_wac->hid_data.id) && !wacom_wac->hid_data.tipswitch;
+
+                       if (wacom_wac->hid_data.confidence || touch_removed) {
+                               wacom_wac_finger_slot(wacom_wac, wacom_wac->touch_input);
+                       }
+               }
        }
 }
 
@@ -2659,6 +2682,10 @@ static void wacom_wac_finger_pre_report(struct hid_device *hdev,
 
        hid_data->confidence = true;
 
+       hid_data->cc_report = 0;
+       hid_data->cc_index = -1;
+       hid_data->cc_value_index = -1;
+
        for (i = 0; i < report->maxfield; i++) {
                struct hid_field *field = report->field[i];
                int j;
@@ -2692,11 +2719,14 @@ static void wacom_wac_finger_pre_report(struct hid_device *hdev,
            hid_data->cc_index >= 0) {
                struct hid_field *field = report->field[hid_data->cc_index];
                int value = field->value[hid_data->cc_value_index];
-               if (value)
+               if (value) {
                        hid_data->num_expected = value;
+                       hid_data->num_received = 0;
+               }
        }
        else {
                hid_data->num_expected = wacom_wac->features.touch_max;
+               hid_data->num_received = 0;
        }
 }
 
@@ -2724,6 +2754,7 @@ static void wacom_wac_finger_report(struct hid_device *hdev,
 
        input_sync(input);
        wacom_wac->hid_data.num_received = 0;
+       wacom_wac->hid_data.num_expected = 0;
 
        /* keep touch state for pen event */
        wacom_wac->shared->touch_down = wacom_wac_finger_count_touches(wacom_wac);