All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] HID: corsair: fix DMA to stack and info leaks
@ 2017-01-12 17:17 ` Johan Hovold
  0 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2017-01-12 17:17 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Benjamin Tissoires, linux-input, linux-usb, linux-kernel, Johan Hovold

These patches fix DMA buffers on stack and information leaks in the
corsair HID driver.

Note that this series has only been compile tested.

Johan


Johan Hovold (2):
  HID: corsair: fix DMA buffers on stack
  HID: corsair: fix control-transfer error handling

 drivers/hid/hid-corsair.c | 60 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 45 insertions(+), 15 deletions(-)

-- 
2.10.2

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 0/2] HID: corsair: fix DMA to stack and info leaks
@ 2017-01-12 17:17 ` Johan Hovold
  0 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2017-01-12 17:17 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Benjamin Tissoires, linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Johan Hovold

These patches fix DMA buffers on stack and information leaks in the
corsair HID driver.

Note that this series has only been compile tested.

Johan


Johan Hovold (2):
  HID: corsair: fix DMA buffers on stack
  HID: corsair: fix control-transfer error handling

 drivers/hid/hid-corsair.c | 60 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 45 insertions(+), 15 deletions(-)

-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 1/2] HID: corsair: fix DMA buffers on stack
  2017-01-12 17:17 ` Johan Hovold
  (?)
@ 2017-01-12 17:17 ` Johan Hovold
  -1 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2017-01-12 17:17 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Benjamin Tissoires, linux-input, linux-usb, linux-kernel,
	Johan Hovold, stable

Not all platforms support DMA to the stack, and specifically since v4.9
this is no longer supported on x86 with VMAP_STACK either.

Note that the macro-mode buffer was larger than necessary.

Fixes: 6f78193ee9ea ("HID: corsair: Add Corsair Vengeance K90 driver")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/hid/hid-corsair.c | 54 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 717704e9ae07..5971907a23b1 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
@@ -148,7 +148,11 @@ static enum led_brightness k90_backlight_get(struct led_classdev *led_cdev)
 	struct usb_interface *usbif = to_usb_interface(dev->parent);
 	struct usb_device *usbdev = interface_to_usbdev(usbif);
 	int brightness;
-	char data[8];
+	char *data;
+
+	data = kmalloc(8, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
 
 	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
 			      K90_REQUEST_STATUS,
@@ -158,16 +162,22 @@ static enum led_brightness k90_backlight_get(struct led_classdev *led_cdev)
 	if (ret < 0) {
 		dev_warn(dev, "Failed to get K90 initial state (error %d).\n",
 			 ret);
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 	brightness = data[4];
 	if (brightness < 0 || brightness > 3) {
 		dev_warn(dev,
 			 "Read invalid backlight brightness: %02hhx.\n",
 			 data[4]);
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
-	return brightness;
+	ret = brightness;
+out:
+	kfree(data);
+
+	return ret;
 }
 
 static enum led_brightness k90_record_led_get(struct led_classdev *led_cdev)
@@ -253,7 +263,11 @@ static ssize_t k90_show_macro_mode(struct device *dev,
 	struct usb_interface *usbif = to_usb_interface(dev->parent);
 	struct usb_device *usbdev = interface_to_usbdev(usbif);
 	const char *macro_mode;
-	char data[8];
+	char *data;
+
+	data = kmalloc(2, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
 
 	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
 			      K90_REQUEST_GET_MODE,
@@ -263,7 +277,8 @@ static ssize_t k90_show_macro_mode(struct device *dev,
 	if (ret < 0) {
 		dev_warn(dev, "Failed to get K90 initial mode (error %d).\n",
 			 ret);
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 
 	switch (data[0]) {
@@ -277,10 +292,15 @@ static ssize_t k90_show_macro_mode(struct device *dev,
 	default:
 		dev_warn(dev, "K90 in unknown mode: %02hhx.\n",
 			 data[0]);
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 
-	return snprintf(buf, PAGE_SIZE, "%s\n", macro_mode);
+	ret = snprintf(buf, PAGE_SIZE, "%s\n", macro_mode);
+out:
+	kfree(data);
+
+	return ret;
 }
 
 static ssize_t k90_store_macro_mode(struct device *dev,
@@ -320,7 +340,11 @@ static ssize_t k90_show_current_profile(struct device *dev,
 	struct usb_interface *usbif = to_usb_interface(dev->parent);
 	struct usb_device *usbdev = interface_to_usbdev(usbif);
 	int current_profile;
-	char data[8];
+	char *data;
+
+	data = kmalloc(8, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
 
 	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
 			      K90_REQUEST_STATUS,
@@ -330,16 +354,22 @@ static ssize_t k90_show_current_profile(struct device *dev,
 	if (ret < 0) {
 		dev_warn(dev, "Failed to get K90 initial state (error %d).\n",
 			 ret);
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 	current_profile = data[7];
 	if (current_profile < 1 || current_profile > 3) {
 		dev_warn(dev, "Read invalid current profile: %02hhx.\n",
 			 data[7]);
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 
-	return snprintf(buf, PAGE_SIZE, "%d\n", current_profile);
+	ret = snprintf(buf, PAGE_SIZE, "%d\n", current_profile);
+out:
+	kfree(data);
+
+	return ret;
 }
 
 static ssize_t k90_store_current_profile(struct device *dev,
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] HID: corsair: fix control-transfer error handling
  2017-01-12 17:17 ` Johan Hovold
  (?)
  (?)
@ 2017-01-12 17:17 ` Johan Hovold
  -1 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2017-01-12 17:17 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Benjamin Tissoires, linux-input, linux-usb, linux-kernel,
	Johan Hovold, stable

Make sure to check for short control transfers in order to avoid parsing
uninitialised buffer data and leaking it to user space.

Note that the backlight and macro-mode buffer constraints are kept as
loose as possible in order to avoid any regressions should the current
buffer sizes be larger than necessary.

Fixes: 6f78193ee9ea ("HID: corsair: Add Corsair Vengeance K90 driver")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/hid/hid-corsair.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 5971907a23b1..c0303f61c26a 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
@@ -159,7 +159,7 @@ static enum led_brightness k90_backlight_get(struct led_classdev *led_cdev)
 			      USB_DIR_IN | USB_TYPE_VENDOR |
 			      USB_RECIP_DEVICE, 0, 0, data, 8,
 			      USB_CTRL_SET_TIMEOUT);
-	if (ret < 0) {
+	if (ret < 5) {
 		dev_warn(dev, "Failed to get K90 initial state (error %d).\n",
 			 ret);
 		ret = -EIO;
@@ -274,7 +274,7 @@ static ssize_t k90_show_macro_mode(struct device *dev,
 			      USB_DIR_IN | USB_TYPE_VENDOR |
 			      USB_RECIP_DEVICE, 0, 0, data, 2,
 			      USB_CTRL_SET_TIMEOUT);
-	if (ret < 0) {
+	if (ret < 1) {
 		dev_warn(dev, "Failed to get K90 initial mode (error %d).\n",
 			 ret);
 		ret = -EIO;
@@ -351,7 +351,7 @@ static ssize_t k90_show_current_profile(struct device *dev,
 			      USB_DIR_IN | USB_TYPE_VENDOR |
 			      USB_RECIP_DEVICE, 0, 0, data, 8,
 			      USB_CTRL_SET_TIMEOUT);
-	if (ret < 0) {
+	if (ret < 8) {
 		dev_warn(dev, "Failed to get K90 initial state (error %d).\n",
 			 ret);
 		ret = -EIO;
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] HID: corsair: fix DMA to stack and info leaks
  2017-01-12 17:17 ` Johan Hovold
                   ` (2 preceding siblings ...)
  (?)
@ 2017-01-13 11:07 ` Jiri Kosina
  -1 siblings, 0 replies; 5+ messages in thread
From: Jiri Kosina @ 2017-01-13 11:07 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Benjamin Tissoires, linux-input, linux-usb, linux-kernel,
	Clément Vuchener

On Thu, 12 Jan 2017, Johan Hovold wrote:

> These patches fix DMA buffers on stack and information leaks in the
> corsair HID driver.
> 
> Note that this series has only been compile tested.

Adding Clément to CC, and applying to for-4.10/upstream-fixes branch.

Thanks!

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-13 11:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-12 17:17 [PATCH 0/2] HID: corsair: fix DMA to stack and info leaks Johan Hovold
2017-01-12 17:17 ` Johan Hovold
2017-01-12 17:17 ` [PATCH 1/2] HID: corsair: fix DMA buffers on stack Johan Hovold
2017-01-12 17:17 ` [PATCH 2/2] HID: corsair: fix control-transfer error handling Johan Hovold
2017-01-13 11:07 ` [PATCH 0/2] HID: corsair: fix DMA to stack and info leaks Jiri Kosina

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.