From 1ffb3c40ffb5c51bc39736409b11816c4260218e Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Mon, 21 Nov 2016 11:48:39 +0100 Subject: [PATCH 1/5] HID: cp2112: make transfer buffers DMA capable Kernel v4.9 strictly enforces DMA capable buffers, so we need to remove buffers allocated on the stack. Use a spinlock to prevent concurrent accesses to the buffer. Signed-off-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/hid-cp2112.c | 115 +++++++++++++++++++++++++++------------ 1 file changed, 79 insertions(+), 36 deletions(-) diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index 086d8a507157..60d30203a5fa 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -32,6 +32,11 @@ #include #include "hid-ids.h" +#define CP2112_REPORT_MAX_LENGTH 64 +#define CP2112_GPIO_CONFIG_LENGTH 5 +#define CP2112_GPIO_GET_LENGTH 2 +#define CP2112_GPIO_SET_LENGTH 3 + enum { CP2112_GPIO_CONFIG = 0x02, CP2112_GPIO_GET = 0x03, @@ -161,6 +166,8 @@ struct cp2112_device { atomic_t read_avail; atomic_t xfer_avail; struct gpio_chip gc; + u8 *in_out_buffer; + spinlock_t lock; }; static int gpio_push_pull = 0xFF; @@ -171,62 +178,86 @@ static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset) { struct cp2112_device *dev = gpiochip_get_data(chip); struct hid_device *hdev = dev->hdev; - u8 buf[5]; + u8 *buf = dev->in_out_buffer; + unsigned long flags; int ret; + spin_lock_irqsave(&dev->lock, flags); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, - sizeof(buf), HID_FEATURE_REPORT, - HID_REQ_GET_REPORT); - if (ret != sizeof(buf)) { + CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, + HID_REQ_GET_REPORT); + if (ret != CP2112_GPIO_CONFIG_LENGTH) { hid_err(hdev, "error requesting GPIO config: %d\n", ret); - return ret; + goto exit; } buf[1] &= ~(1 << offset); buf[2] = gpio_push_pull; - ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, + CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, + HID_REQ_SET_REPORT); if (ret < 0) { hid_err(hdev, "error setting GPIO config: %d\n", ret); - return ret; + goto exit; } - return 0; + ret = 0; + +exit: + spin_unlock_irqrestore(&dev->lock, flags); + return ret <= 0 ? ret : -EIO; } static void cp2112_gpio_set(struct gpio_chip *chip, unsigned offset, int value) { struct cp2112_device *dev = gpiochip_get_data(chip); struct hid_device *hdev = dev->hdev; - u8 buf[3]; + u8 *buf = dev->in_out_buffer; + unsigned long flags; int ret; + spin_lock_irqsave(&dev->lock, flags); + buf[0] = CP2112_GPIO_SET; buf[1] = value ? 0xff : 0; buf[2] = 1 << offset; - ret = hid_hw_raw_request(hdev, CP2112_GPIO_SET, buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_SET, buf, + CP2112_GPIO_SET_LENGTH, HID_FEATURE_REPORT, + HID_REQ_SET_REPORT); if (ret < 0) hid_err(hdev, "error setting GPIO values: %d\n", ret); + + spin_unlock_irqrestore(&dev->lock, flags); } static int cp2112_gpio_get(struct gpio_chip *chip, unsigned offset) { struct cp2112_device *dev = gpiochip_get_data(chip); struct hid_device *hdev = dev->hdev; - u8 buf[2]; + u8 *buf = dev->in_out_buffer; + unsigned long flags; int ret; - ret = hid_hw_raw_request(hdev, CP2112_GPIO_GET, buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_GET_REPORT); - if (ret != sizeof(buf)) { + spin_lock_irqsave(&dev->lock, flags); + + ret = hid_hw_raw_request(hdev, CP2112_GPIO_GET, buf, + CP2112_GPIO_GET_LENGTH, HID_FEATURE_REPORT, + HID_REQ_GET_REPORT); + if (ret != CP2112_GPIO_GET_LENGTH) { hid_err(hdev, "error requesting GPIO values: %d\n", ret); - return ret; + ret = ret < 0 ? ret : -EIO; + goto exit; } - return (buf[1] >> offset) & 1; + ret = (buf[1] >> offset) & 1; + +exit: + spin_unlock_irqrestore(&dev->lock, flags); + + return ret; } static int cp2112_gpio_direction_output(struct gpio_chip *chip, @@ -234,27 +265,33 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, { struct cp2112_device *dev = gpiochip_get_data(chip); struct hid_device *hdev = dev->hdev; - u8 buf[5]; + u8 *buf = dev->in_out_buffer; + unsigned long flags; int ret; + spin_lock_irqsave(&dev->lock, flags); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, - sizeof(buf), HID_FEATURE_REPORT, - HID_REQ_GET_REPORT); - if (ret != sizeof(buf)) { + CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, + HID_REQ_GET_REPORT); + if (ret != CP2112_GPIO_CONFIG_LENGTH) { hid_err(hdev, "error requesting GPIO config: %d\n", ret); - return ret; + goto fail; } buf[1] |= 1 << offset; buf[2] = gpio_push_pull; - ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, + CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, + HID_REQ_SET_REPORT); if (ret < 0) { hid_err(hdev, "error setting GPIO config: %d\n", ret); - return ret; + goto fail; } + spin_unlock_irqrestore(&dev->lock, flags); + /* * Set gpio value when output direction is already set, * as specified in AN495, Rev. 0.2, cpt. 4.4 @@ -262,6 +299,10 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, cp2112_gpio_set(chip, offset, value); return 0; + +fail: + spin_unlock_irqrestore(&dev->lock, flags); + return ret < 0 ? ret : -EIO; } static int cp2112_hid_get(struct hid_device *hdev, unsigned char report_number, @@ -1007,6 +1048,17 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) struct cp2112_smbus_config_report config; int ret; + dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL); + if (!dev) + return -ENOMEM; + + dev->in_out_buffer = devm_kzalloc(&hdev->dev, CP2112_REPORT_MAX_LENGTH, + GFP_KERNEL); + if (!dev->in_out_buffer) + return -ENOMEM; + + spin_lock_init(&dev->lock); + ret = hid_parse(hdev); if (ret) { hid_err(hdev, "parse failed\n"); @@ -1063,12 +1115,6 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) goto err_power_normal; } - dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (!dev) { - ret = -ENOMEM; - goto err_power_normal; - } - hid_set_drvdata(hdev, (void *)dev); dev->hdev = hdev; dev->adap.owner = THIS_MODULE; @@ -1087,7 +1133,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) if (ret) { hid_err(hdev, "error registering i2c adapter\n"); - goto err_free_dev; + goto err_power_normal; } hid_dbg(hdev, "adapter registered\n"); @@ -1123,8 +1169,6 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) gpiochip_remove(&dev->gc); err_free_i2c: i2c_del_adapter(&dev->adap); -err_free_dev: - kfree(dev); err_power_normal: hid_hw_power(hdev, PM_HINT_NORMAL); err_hid_close: @@ -1149,7 +1193,6 @@ static void cp2112_remove(struct hid_device *hdev) */ hid_hw_close(hdev); hid_hw_stop(hdev); - kfree(dev); } static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report, From 061232f0d47fa10103f3efa3e890f002a930d902 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Mon, 21 Nov 2016 11:48:40 +0100 Subject: [PATCH 2/5] HID: lg: make transfer buffers DMA capable Kernel v4.9 strictly enforces DMA capable buffers, so we need to remove buffers allocated on the stack. [jkosina@suse.cz: fix up second usage of hid_hw_raw_request(), spotted by 0day build bot] Signed-off-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/hid-lg.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c index 76f644deb0a7..c5c5fbe9d605 100644 --- a/drivers/hid/hid-lg.c +++ b/drivers/hid/hid-lg.c @@ -756,11 +756,16 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id) /* Setup wireless link with Logitech Wii wheel */ if (hdev->product == USB_DEVICE_ID_LOGITECH_WII_WHEEL) { - unsigned char buf[] = { 0x00, 0xAF, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; + const unsigned char cbuf[] = { 0x00, 0xAF, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; + u8 *buf = kmemdup(cbuf, sizeof(cbuf), GFP_KERNEL); - ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf), + if (!buf) { + ret = -ENOMEM; + goto err_free; + } + + ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(cbuf), HID_FEATURE_REPORT, HID_REQ_SET_REPORT); - if (ret >= 0) { /* insert a little delay of 10 jiffies ~ 40ms */ wait_queue_head_t wait; @@ -772,9 +777,10 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id) buf[1] = 0xB2; get_random_bytes(&buf[2], 2); - ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf), + ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(cbuf), HID_FEATURE_REPORT, HID_REQ_SET_REPORT); } + kfree(buf); } if (drv_data->quirks & LG_FF) From b7a87ad6775f3ed69e6573b91ed3c2f1338884ad Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Mon, 21 Nov 2016 11:48:41 +0100 Subject: [PATCH 3/5] HID: magicmouse: make transfer buffers DMA capable Kernel v4.9 strictly enforces DMA capable buffers, so we need to remove buffers allocated on the stack. Signed-off-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/hid-magicmouse.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index d6fa496d0ca2..20b40ad26325 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -493,7 +493,8 @@ static int magicmouse_input_configured(struct hid_device *hdev, static int magicmouse_probe(struct hid_device *hdev, const struct hid_device_id *id) { - __u8 feature[] = { 0xd7, 0x01 }; + const u8 feature[] = { 0xd7, 0x01 }; + u8 *buf; struct magicmouse_sc *msc; struct hid_report *report; int ret; @@ -544,6 +545,12 @@ static int magicmouse_probe(struct hid_device *hdev, } report->size = 6; + buf = kmemdup(feature, sizeof(feature), GFP_KERNEL); + if (!buf) { + ret = -ENOMEM; + goto err_stop_hw; + } + /* * Some devices repond with 'invalid report id' when feature * report switching it into multitouch mode is sent to it. @@ -552,8 +559,9 @@ static int magicmouse_probe(struct hid_device *hdev, * but there seems to be no other way of switching the mode. * Thus the super-ugly hacky success check below. */ - ret = hid_hw_raw_request(hdev, feature[0], feature, sizeof(feature), + ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(feature), HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + kfree(buf); if (ret != -EIO && ret != sizeof(feature)) { hid_err(hdev, "unable to request touch data (%d)\n", ret); goto err_stop_hw; From 6dab07df555b652d8d989348b2ce04498d7f9a70 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Mon, 21 Nov 2016 11:48:42 +0100 Subject: [PATCH 4/5] HID: rmi: make transfer buffers DMA capable Kernel v4.9 strictly enforces DMA capable buffers, so we need to remove buffers allocated on the stack. Signed-off-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/hid-rmi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c index 9cd2ca34a6be..be89bcbf6a71 100644 --- a/drivers/hid/hid-rmi.c +++ b/drivers/hid/hid-rmi.c @@ -188,10 +188,16 @@ static int rmi_set_page(struct hid_device *hdev, u8 page) static int rmi_set_mode(struct hid_device *hdev, u8 mode) { int ret; - u8 txbuf[2] = {RMI_SET_RMI_MODE_REPORT_ID, mode}; + const u8 txbuf[2] = {RMI_SET_RMI_MODE_REPORT_ID, mode}; + u8 *buf; - ret = hid_hw_raw_request(hdev, RMI_SET_RMI_MODE_REPORT_ID, txbuf, + buf = kmemdup(txbuf, sizeof(txbuf), GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = hid_hw_raw_request(hdev, RMI_SET_RMI_MODE_REPORT_ID, buf, sizeof(txbuf), HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + kfree(buf); if (ret < 0) { dev_err(&hdev->dev, "unable to set rmi mode to %d (%d)\n", mode, ret); From d443a0aa3a291e5f78072f2fa464e03bc83fafad Mon Sep 17 00:00:00 2001 From: Song Hongyan Date: Tue, 15 Nov 2016 01:11:10 +0000 Subject: [PATCH 5/5] HID: hid-sensor-hub: clear memory to avoid random data When user tried to read some fields like hysteresis from IIO sysfs on some systems, it fails. The reason is that this field is a byte field and caller of sensor_hub_get_feature() passes a buffer of 4 bytes. Here the function sensor_hub_get_feature() copies the single byte from the report to the caller buffer and returns "1" as the number of bytes copied. So caller can use the return value. But this is done by multiple callers, so if we just change the sensor_hub_get_feature so that caller buffer is initialized with 0s then we don't to change all functions. Signed-off-by: Song Hongyan Acked-by: Jonathan Cameron Signed-off-by: Jiri Kosina --- drivers/hid/hid-sensor-hub.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c index c5c3d6111729..60875625cbdf 100644 --- a/drivers/hid/hid-sensor-hub.c +++ b/drivers/hid/hid-sensor-hub.c @@ -212,6 +212,7 @@ int sensor_hub_set_feature(struct hid_sensor_hub_device *hsdev, u32 report_id, __s32 value; int ret = 0; + memset(buffer, 0, buffer_size); mutex_lock(&data->mutex); report = sensor_hub_report(report_id, hsdev->hdev, HID_FEATURE_REPORT); if (!report || (field_index >= report->maxfield)) {