[PATCH v3 00/10] HID: validate report details

[PATCH v3 00/10] HID: validate report details

[Benjamin Tissoires]

Hi guys,

here is the v3 of the CVE fixes.

I have tested the multitouch and logitech-dj part, and the lenovo-tpkbd has been
tested in the bug referenced in patch 10.

Cheers,
Benjamin

Changes since v2:
- fix lenovo-tpkbd report validation
- fix lenovo-tpkbd not releasing the device when the report was not valid
- use generic tests found in previous hid-multitouch patches, so that this will
  not happen again
- fix input_report index retrieving in hid-multitouch

Original message from Kees (v2):

These patches introduce a validation function for HID devices that do
direct report value accesses, solving a number of heap smashing flaws.

This version changes to using an field-index-based checker for the new
"hid_validate_values()" which requires callers to loop across fields if
they use more than one field.

Benjamin Tissoires (3):
  HID: validate feature and input report details
  HID: multitouch: validate indexes details
  HID: lenovo-tpkbd: fix leak if tpkbd_probe_tp fails

Kees Cook (7):
  HID: provide a helper for validating hid reports
  HID: zeroplus: validate output report details
  HID: sony: validate HID output report details
  HID: steelseries: validate output report details
  HID: LG: validate HID output report details
  HID: lenovo-tpkbd: validate output report details
  HID: logitech-dj: validate output report details

 drivers/hid/hid-core.c         | 74 +++++++++++++++++++++++++++++++++++++-----
 drivers/hid/hid-input.c        | 11 ++++++-
 drivers/hid/hid-lenovo-tpkbd.c | 25 ++++++++++----
 drivers/hid/hid-lg2ff.c        | 19 ++---------
 drivers/hid/hid-lg3ff.c        | 29 ++++-------------
 drivers/hid/hid-lg4ff.c        | 20 +-----------
 drivers/hid/hid-lgff.c         | 17 ++--------
 drivers/hid/hid-logitech-dj.c  | 10 ++++--
 drivers/hid/hid-multitouch.c   | 26 ++++++++-------
 drivers/hid/hid-sony.c         |  4 +++
 drivers/hid/hid-steelseries.c  |  5 +++
 drivers/hid/hid-zpff.c         | 18 +++-------
 include/linux/hid.h            |  4 +++
 13 files changed, 146 insertions(+), 116 deletions(-)

-- 
1.8.3.1

[PATCH v3 01/10] HID: provide a helper for validating hid reports

[Benjamin Tissoires]

From: Kees Cook <keescook@chromium.org>

Many drivers need to validate the characteristics of their HID report
during initialization to avoid misusing the reports. This adds a common
helper to perform validation of the report exisitng, the field existing,
and the expected number of values within the field.

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
 - no changes

v2:
 - suggestions from Benjamin Tissoires:
  - check id too, just to be double-safe.
  - updated to check a specific field, moving the for loop to callers.

 drivers/hid/hid-core.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/hid.h    |  4 ++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 2c77854..44b6c68 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -801,6 +801,64 @@ int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size)
 }
 EXPORT_SYMBOL_GPL(hid_parse_report);
 
+static const char * const hid_report_names[] = {
+	"HID_INPUT_REPORT",
+	"HID_OUTPUT_REPORT",
+	"HID_FEATURE_REPORT",
+};
+/**
+ * hid_validate_values - validate existing device report's value indexes
+ *
+ * @device: hid device
+ * @type: which report type to examine
+ * @id: which report ID to examine (0 for first)
+ * @field_index: which report field to examine
+ * @report_counts: expected number of values
+ *
+ * Validate the number of values in a given field of a given report, after
+ * parsing.
+ */
+struct hid_report *hid_validate_values(struct hid_device *hid,
+				       unsigned int type, unsigned int id,
+				       unsigned int field_index,
+				       unsigned int report_counts)
+{
+	struct hid_report *report;
+
+	if (type > HID_FEATURE_REPORT) {
+		hid_err(hid, "invalid HID report type %u\n", type);
+		return NULL;
+	}
+
+	if (id >= HID_MAX_IDS) {
+		hid_err(hid, "invalid HID report id %u\n", id);
+		return NULL;
+	}
+
+	/*
+	 * Explicitly not using hid_get_report() here since it depends on
+	 * ->numbered being checked, which may not always be the case when
+	 * drivers go to access report values.
+	 */
+	report = hid->report_enum[type].report_id_hash[id];
+	if (!report) {
+		hid_err(hid, "missing %s %u\n", hid_report_names[type], id);
+		return NULL;
+	}
+	if (report->maxfield <= field_index) {
+		hid_err(hid, "not enough fields in %s %u\n",
+			hid_report_names[type], id);
+		return NULL;
+	}
+	if (report->field[field_index]->report_count < report_counts) {
+		hid_err(hid, "not enough values in %s %u field %u\n",
+			hid_report_names[type], id, field_index);
+		return NULL;
+	}
+	return report;
+}
+EXPORT_SYMBOL_GPL(hid_validate_values);
+
 /**
  * hid_open_report - open a driver-specific device report
  *
diff --git a/include/linux/hid.h b/include/linux/hid.h
index ee1ffc5..31b9d29 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -756,6 +756,10 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags);
 struct hid_device *hid_allocate_device(void);
 struct hid_report *hid_register_report(struct hid_device *device, unsigned type, unsigned id);
 int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size);
+struct hid_report *hid_validate_values(struct hid_device *hid,
+				       unsigned int type, unsigned int id,
+				       unsigned int field_index,
+				       unsigned int report_counts);
 int hid_open_report(struct hid_device *device);
 int hid_check_keys_pressed(struct hid_device *hid);
 int hid_connect(struct hid_device *hid, unsigned int connect_mask);
-- 
1.8.3.1

[PATCH v3 02/10] HID: zeroplus: validate output report details

[Benjamin Tissoires]

From: Kees Cook <keescook@chromium.org>

The zeroplus HID driver was not checking the size of allocated values
in fields it used. A HID device could send a malicious output report
that would cause the driver to write beyond the output report allocation
during initialization, causing a heap overflow:

[ 1442.728680] usb 1-1: New USB device found, idVendor=0c12, idProduct=0005
...
[ 1466.243173] BUG kmalloc-192 (Tainted: G        W   ): Redzone overwritten

CVE-2013-2889

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
 - no changes

 drivers/hid/hid-zpff.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-zpff.c b/drivers/hid/hid-zpff.c
index 6ec28a3..a29756c 100644
--- a/drivers/hid/hid-zpff.c
+++ b/drivers/hid/hid-zpff.c
@@ -68,21 +68,13 @@ static int zpff_init(struct hid_device *hid)
 	struct hid_report *report;
 	struct hid_input *hidinput = list_entry(hid->inputs.next,
 						struct hid_input, list);
-	struct list_head *report_list =
-			&hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct input_dev *dev = hidinput->input;
-	int error;
+	int i, error;
 
-	if (list_empty(report_list)) {
-		hid_err(hid, "no output report found\n");
-		return -ENODEV;
-	}
-
-	report = list_entry(report_list->next, struct hid_report, list);
-
-	if (report->maxfield < 4) {
-		hid_err(hid, "not enough fields in report\n");
-		return -ENODEV;
+	for (i = 0; i < 4; i++) {
+		report = hid_validate_values(hid, HID_OUTPUT_REPORT, 0, i, 1);
+		if (!report)
+			return -ENODEV;
 	}
 
 	zpff = kzalloc(sizeof(struct zpff_device), GFP_KERNEL);
-- 
1.8.3.1

[PATCH v3 03/10] HID: sony: validate HID output report details

[Benjamin Tissoires]

From: Kees Cook <keescook@chromium.org>

This driver must validate the availability of the HID output report and
its size before it can write LED states via buzz_set_leds(). This stops
a heap overflow that is possible if a device provides a malicious HID
output report:

[  108.171280] usb 1-1: New USB device found, idVendor=054c, idProduct=0002
...
[  117.507877] BUG kmalloc-192 (Not tainted): Redzone overwritten

CVE-2013-2890

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
 - no changes

 drivers/hid/hid-sony.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 30dbb6b..b18320d 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -537,6 +537,10 @@ static int buzz_init(struct hid_device *hdev)
 	drv_data = hid_get_drvdata(hdev);
 	BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
 
+	/* Validate expected report characteristics. */
+	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
+		return -ENODEV;
+
 	buzz = kzalloc(sizeof(*buzz), GFP_KERNEL);
 	if (!buzz) {
 		hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
-- 
1.8.3.1

[PATCH v3 04/10] HID: steelseries: validate output report details

[Benjamin Tissoires]

From: Kees Cook <keescook@chromium.org>

A HID device could send a malicious output report that would cause the
steelseries HID driver to write beyond the output report allocation
during initialization, causing a heap overflow:

[  167.981534] usb 1-1: New USB device found, idVendor=1038, idProduct=1410
...
[  182.050547] BUG kmalloc-256 (Tainted: G        W   ): Redzone overwritten

CVE-2013-2891

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
 - no changes

 drivers/hid/hid-steelseries.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index d164911..29f328f 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -249,6 +249,11 @@ static int steelseries_srws1_probe(struct hid_device *hdev,
 		goto err_free;
 	}
 
+	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 16)) {
+		ret = -ENODEV;
+		goto err_free;
+	}
+
 	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 	if (ret) {
 		hid_err(hdev, "hw start failed\n");
-- 
1.8.3.1

[PATCH v3 05/10] HID: LG: validate HID output report details

[Benjamin Tissoires]

From: Kees Cook <keescook@chromium.org>

A HID device could send a malicious output report that would cause the
lg, lg3, and lg4 HID drivers to write beyond the output report allocation
during an event, causing a heap overflow:

[  325.245240] usb 1-1: New USB device found, idVendor=046d, idProduct=c287
...
[  414.518960] BUG kmalloc-4096 (Not tainted): Redzone overwritten

Additionally, while lg2 did correctly validate the report details, it was
cleaned up and shortened.

CVE-2013-2893

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
 - no changes

 drivers/hid/hid-lg2ff.c | 19 +++----------------
 drivers/hid/hid-lg3ff.c | 29 ++++++-----------------------
 drivers/hid/hid-lg4ff.c | 20 +-------------------
 drivers/hid/hid-lgff.c  | 17 ++---------------
 4 files changed, 12 insertions(+), 73 deletions(-)

diff --git a/drivers/hid/hid-lg2ff.c b/drivers/hid/hid-lg2ff.c
index b3cd150..1a42eaa 100644
--- a/drivers/hid/hid-lg2ff.c
+++ b/drivers/hid/hid-lg2ff.c
@@ -64,26 +64,13 @@ int lg2ff_init(struct hid_device *hid)
 	struct hid_report *report;
 	struct hid_input *hidinput = list_entry(hid->inputs.next,
 						struct hid_input, list);
-	struct list_head *report_list =
-			&hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct input_dev *dev = hidinput->input;
 	int error;
 
-	if (list_empty(report_list)) {
-		hid_err(hid, "no output report found\n");
+	/* Check that the report looks ok */
+	report = hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7);
+	if (!report)
 		return -ENODEV;
-	}
-
-	report = list_entry(report_list->next, struct hid_report, list);
-
-	if (report->maxfield < 1) {
-		hid_err(hid, "output report is empty\n");
-		return -ENODEV;
-	}
-	if (report->field[0]->report_count < 7) {
-		hid_err(hid, "not enough values in the field\n");
-		return -ENODEV;
-	}
 
 	lg2ff = kmalloc(sizeof(struct lg2ff_device), GFP_KERNEL);
 	if (!lg2ff)
diff --git a/drivers/hid/hid-lg3ff.c b/drivers/hid/hid-lg3ff.c
index e52f181..8c2da18 100644
--- a/drivers/hid/hid-lg3ff.c
+++ b/drivers/hid/hid-lg3ff.c
@@ -66,10 +66,11 @@ static int hid_lg3ff_play(struct input_dev *dev, void *data,
 	int x, y;
 
 /*
- * Maxusage should always be 63 (maximum fields)
- * likely a better way to ensure this data is clean
+ * Available values in the field should always be 63, but we only use up to
+ * 35. Instead, clear the entire area, however big it is.
  */
-	memset(report->field[0]->value, 0, sizeof(__s32)*report->field[0]->maxusage);
+	memset(report->field[0]->value, 0,
+	       sizeof(__s32) * report->field[0]->report_count);
 
 	switch (effect->type) {
 	case FF_CONSTANT:
@@ -129,32 +130,14 @@ static const signed short ff3_joystick_ac[] = {
 int lg3ff_init(struct hid_device *hid)
 {
 	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct input_dev *dev = hidinput->input;
-	struct hid_report *report;
-	struct hid_field *field;
 	const signed short *ff_bits = ff3_joystick_ac;
 	int error;
 	int i;
 
-	/* Find the report to use */
-	if (list_empty(report_list)) {
-		hid_err(hid, "No output report found\n");
-		return -1;
-	}
-
 	/* Check that the report looks ok */
-	report = list_entry(report_list->next, struct hid_report, list);
-	if (!report) {
-		hid_err(hid, "NULL output report\n");
-		return -1;
-	}
-
-	field = report->field[0];
-	if (!field) {
-		hid_err(hid, "NULL field\n");
-		return -1;
-	}
+	if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 35))
+		return -ENODEV;
 
 	/* Assume single fixed device G940 */
 	for (i = 0; ff_bits[i] >= 0; i++)
diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 0ddae2a..8782fe1 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -484,34 +484,16 @@ static enum led_brightness lg4ff_led_get_brightness(struct led_classdev *led_cde
 int lg4ff_init(struct hid_device *hid)
 {
 	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct input_dev *dev = hidinput->input;
-	struct hid_report *report;
-	struct hid_field *field;
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
 	struct usb_device_descriptor *udesc;
 	int error, i, j;
 	__u16 bcdDevice, rev_maj, rev_min;
 
-	/* Find the report to use */
-	if (list_empty(report_list)) {
-		hid_err(hid, "No output report found\n");
-		return -1;
-	}
-
 	/* Check that the report looks ok */
-	report = list_entry(report_list->next, struct hid_report, list);
-	if (!report) {
-		hid_err(hid, "NULL output report\n");
+	if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7))
 		return -1;
-	}
-
-	field = report->field[0];
-	if (!field) {
-		hid_err(hid, "NULL field\n");
-		return -1;
-	}
 
 	/* Check what wheel has been connected */
 	for (i = 0; i < ARRAY_SIZE(lg4ff_devices); i++) {
diff --git a/drivers/hid/hid-lgff.c b/drivers/hid/hid-lgff.c
index d7ea8c8..e1394af 100644
--- a/drivers/hid/hid-lgff.c
+++ b/drivers/hid/hid-lgff.c
@@ -128,27 +128,14 @@ static void hid_lgff_set_autocenter(struct input_dev *dev, u16 magnitude)
 int lgff_init(struct hid_device* hid)
 {
 	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct input_dev *dev = hidinput->input;
-	struct hid_report *report;
-	struct hid_field *field;
 	const signed short *ff_bits = ff_joystick;
 	int error;
 	int i;
 
-	/* Find the report to use */
-	if (list_empty(report_list)) {
-		hid_err(hid, "No output report found\n");
-		return -1;
-	}
-
 	/* Check that the report looks ok */
-	report = list_entry(report_list->next, struct hid_report, list);
-	field = report->field[0];
-	if (!field) {
-		hid_err(hid, "NULL field\n");
-		return -1;
-	}
+	if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7))
+		return -ENODEV;
 
 	for (i = 0; i < ARRAY_SIZE(devices); i++) {
 		if (dev->id.vendor == devices[i].idVendor &&
-- 
1.8.3.1

[PATCH v3 06/10] HID: lenovo-tpkbd: validate output report details

[Benjamin Tissoires]

From: Kees Cook <keescook@chromium.org>

From: Kees Cook <keescook@chromium.org>

A HID device could send a malicious output report that would cause the
lenovo-tpkbd HID driver to write just beyond the output report allocation
during initialization, causing a heap overflow:

[   76.109807] usb 1-1: New USB device found, idVendor=17ef, idProduct=6009
...
[   80.462540] BUG kmalloc-192 (Tainted: G        W   ): Redzone overwritten

CVE-2013-2894

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
 - fix feature report check for report ID 4

 drivers/hid/hid-lenovo-tpkbd.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c
index 07837f5..762d988 100644
--- a/drivers/hid/hid-lenovo-tpkbd.c
+++ b/drivers/hid/hid-lenovo-tpkbd.c
@@ -339,7 +339,15 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
 	struct tpkbd_data_pointer *data_pointer;
 	size_t name_sz = strlen(dev_name(dev)) + 16;
 	char *name_mute, *name_micmute;
-	int ret;
+	int i, ret;
+
+	/* Validate required reports. */
+	for (i = 0; i < 4; i++) {
+		if (!hid_validate_values(hdev, HID_FEATURE_REPORT, 4, i, 1))
+			return -ENODEV;
+	}
+	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 3, 0, 2))
+		return -ENODEV;
 
 	if (sysfs_create_group(&hdev->dev.kobj,
 				&tpkbd_attr_group_pointer)) {
-- 
1.8.3.1

[PATCH v3 07/10] HID: logitech-dj: validate output report details

[Benjamin Tissoires]

From: Kees Cook <keescook@chromium.org>

A HID device could send a malicious output report that would cause the
logitech-dj HID driver to leak kernel memory contents to the device, or
trigger a NULL dereference during initialization:

[  304.424553] usb 1-1: New USB device found, idVendor=046d, idProduct=c52b
...
[  304.780467] BUG: unable to handle kernel NULL pointer dereference at 0000000000000028
[  304.781409] IP: [<ffffffff815d50aa>] logi_dj_recv_send_report.isra.11+0x1a/0x90

CVE-2013-2895

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
---
v3:
 - check for the whole size of the DJ report, as per the spec

 drivers/hid/hid-logitech-dj.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 7800b14..2e53024 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -461,7 +461,7 @@ static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev,
 	struct hid_report *report;
 	struct hid_report_enum *output_report_enum;
 	u8 *data = (u8 *)(&dj_report->device_index);
-	int i;
+	unsigned int i;
 
 	output_report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
 	report = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
@@ -471,7 +471,7 @@ static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev,
 		return -ENODEV;
 	}
 
-	for (i = 0; i < report->field[0]->report_count; i++)
+	for (i = 0; i < DJREPORT_SHORT_LENGTH - 1; i++)
 		report->field[0]->value[i] = data[i];
 
 	hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
@@ -791,6 +791,12 @@ static int logi_dj_probe(struct hid_device *hdev,
 		goto hid_parse_fail;
 	}
 
+	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, REPORT_ID_DJ_SHORT,
+				 0, DJREPORT_SHORT_LENGTH - 1)) {
+		retval = -ENODEV;
+		goto hid_parse_fail;
+	}
+
 	/* Starts the usb device and connects to upper interfaces hiddev and
 	 * hidraw */
 	retval = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-- 
1.8.3.1

[PATCH v3 08/10] HID: validate feature and input report details

[Benjamin Tissoires]

When dealing with usage_index, be sure to properly use unsigned instead of
int to avoid overflows.

When working on report fields, always validate that their report_counts are
in bounds.
Without this, a HID device could report a malicious feature report that
could trick the driver into a heap overflow:

[  634.885003] usb 1-1: New USB device found, idVendor=0596, idProduct=0500
...
[  676.469629] BUG kmalloc-192 (Tainted: G        W   ): Redzone overwritten

CVE-2013-2897

Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
 - new patch: extract from the hid-multitouch patch, the generic checks so that
   every hid drivers will benefit from them

 drivers/hid/hid-core.c  | 16 +++++++---------
 drivers/hid/hid-input.c | 11 ++++++++++-
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 44b6c68..329e24e 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -94,7 +94,6 @@ EXPORT_SYMBOL_GPL(hid_register_report);
 static struct hid_field *hid_register_field(struct hid_report *report, unsigned usages, unsigned values)
 {
 	struct hid_field *field;
-	int i;
 
 	if (report->maxfield == HID_MAX_FIELDS) {
 		hid_err(report->device, "too many fields in report\n");
@@ -113,9 +112,6 @@ static struct hid_field *hid_register_field(struct hid_report *report, unsigned
 	field->value = (s32 *)(field->usage + usages);
 	field->report = report;
 
-	for (i = 0; i < usages; i++)
-		field->usage[i].usage_index = i;
-
 	return field;
 }
 
@@ -226,9 +222,9 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
 {
 	struct hid_report *report;
 	struct hid_field *field;
-	int usages;
+	unsigned usages;
 	unsigned offset;
-	int i;
+	unsigned i;
 
 	report = hid_register_report(parser->device, report_type, parser->global.report_id);
 	if (!report) {
@@ -255,7 +251,8 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
 	if (!parser->local.usage_index) /* Ignore padding fields */
 		return 0;
 
-	usages = max_t(int, parser->local.usage_index, parser->global.report_count);
+	usages = max_t(unsigned, parser->local.usage_index,
+				 parser->global.report_count);
 
 	field = hid_register_field(report, usages, parser->global.report_count);
 	if (!field)
@@ -266,13 +263,14 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
 	field->application = hid_lookup_collection(parser, HID_COLLECTION_APPLICATION);
 
 	for (i = 0; i < usages; i++) {
-		int j = i;
+		unsigned j = i;
 		/* Duplicate the last usage we parsed if we have excess values */
 		if (i >= parser->local.usage_index)
 			j = parser->local.usage_index - 1;
 		field->usage[i].hid = parser->local.usage[j];
 		field->usage[i].collection_index =
 			parser->local.collection_index[j];
+		field->usage[i].usage_index = i;
 	}
 
 	field->maxusage = usages;
@@ -1354,7 +1352,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
 			goto out;
 	}
 
-	if (hid->claimed != HID_CLAIMED_HIDRAW) {
+	if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
 		for (a = 0; a < report->maxfield; a++)
 			hid_input_field(hid, report->field[a], cdata, interrupt);
 		hdrv = hid->driver;
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index b420f4a..8741d95 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -485,6 +485,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 	if (field->flags & HID_MAIN_ITEM_CONSTANT)
 		goto ignore;
 
+	/* Ignore if report count is out of bounds. */
+	if (field->report_count < 1)
+		goto ignore;
+
 	/* only LED usages are supported in output fields */
 	if (field->report_type == HID_OUTPUT_REPORT &&
 			(usage->hid & HID_USAGE_PAGE) != HID_UP_LED) {
@@ -1236,7 +1240,11 @@ static void report_features(struct hid_device *hid)
 
 	rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
 	list_for_each_entry(rep, &rep_enum->report_list, list)
-		for (i = 0; i < rep->maxfield; i++)
+		for (i = 0; i < rep->maxfield; i++) {
+			/* Ignore if report count is out of bounds. */
+			if (rep->field[i]->report_count < 1)
+				continue;
+
 			for (j = 0; j < rep->field[i]->maxusage; j++) {
 				/* Verify if Battery Strength feature is available */
 				hidinput_setup_battery(hid, HID_FEATURE_REPORT, rep->field[i]);
@@ -1245,6 +1253,7 @@ static void report_features(struct hid_device *hid)
 					drv->feature_mapping(hid, rep->field[i],
 							     rep->field[i]->usage + j);
 			}
+		}
 }
 
 static struct hid_input *hidinput_allocate(struct hid_device *hid)
-- 
1.8.3.1

[PATCH v3 09/10] HID: multitouch: validate indexes details

[Benjamin Tissoires]

When working on report indexes, always validate that they are in bounds.
Without this, a HID device could report a malicious feature report that
could trick the driver into a heap overflow:

[  634.885003] usb 1-1: New USB device found, idVendor=0596, idProduct=0500
...
[  676.469629] BUG kmalloc-192 (Tainted: G        W   ): Redzone overwritten

Note that we need to change the indexes from s8 to s16 as they can
be between -1 and 255.

CVE-2013-2897

Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
 - extract from hid-multitouch the generic checks so that every hid drivers will
   benefit from them
 - change __s8 index declarations into __s16
 - use usage_index for the input_mode index instead of a half working code
 - check the indexes validities only once
 
 drivers/hid/hid-multitouch.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index ac28f08..5e5fe1b 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -101,9 +101,9 @@ struct mt_device {
 	unsigned last_slot_field;	/* the last field of a slot */
 	unsigned mt_report_id;	/* the report ID of the multitouch device */
 	unsigned pen_report_id;	/* the report ID of the pen device */
-	__s8 inputmode;		/* InputMode HID feature, -1 if non-existent */
-	__s8 inputmode_index;	/* InputMode HID feature index in the report */
-	__s8 maxcontact_report_id;	/* Maximum Contact Number HID feature,
+	__s16 inputmode;	/* InputMode HID feature, -1 if non-existent */
+	__s16 inputmode_index;	/* InputMode HID feature index in the report */
+	__s16 maxcontact_report_id;	/* Maximum Contact Number HID feature,
 				   -1 if non-existent */
 	__u8 num_received;	/* how many contacts we received */
 	__u8 num_expected;	/* expected last contact index */
@@ -312,20 +312,18 @@ static void mt_feature_mapping(struct hid_device *hdev,
 		struct hid_field *field, struct hid_usage *usage)
 {
 	struct mt_device *td = hid_get_drvdata(hdev);
-	int i;
 
 	switch (usage->hid) {
 	case HID_DG_INPUTMODE:
-		td->inputmode = field->report->id;
-		td->inputmode_index = 0; /* has to be updated below */
-
-		for (i=0; i < field->maxusage; i++) {
-			if (field->usage[i].hid == usage->hid) {
-				td->inputmode_index = i;
-				break;
-			}
+		/* Ignore if value index is out of bounds. */
+		if (usage->usage_index >= field->report_count) {
+			dev_err(&hdev->dev, "HID_DG_INPUTMODE out of range\n");
+			break;
 		}
 
+		td->inputmode = field->report->id;
+		td->inputmode_index = usage->usage_index;
+
 		break;
 	case HID_DG_CONTACTMAX:
 		td->maxcontact_report_id = field->report->id;
@@ -511,6 +509,10 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			mt_store_field(usage, td, hi);
 			return 1;
 		case HID_DG_CONTACTCOUNT:
+			/* Ignore if indexes are out of bounds. */
+			if (field->index >= field->report->maxfield ||
+			    usage->usage_index >= field->report_count)
+				return 1;
 			td->cc_index = field->index;
 			td->cc_value_index = usage->usage_index;
 			return 1;
-- 
1.8.3.1

[PATCH v3 10/10] HID: lenovo-tpkbd: fix leak if tpkbd_probe_tp fails

[Benjamin Tissoires]

If tpkbd_probe_tp() bails out, the probe() function return an error,
but hid_hw_stop() is never called.

fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1003998

Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
 - new patch

 drivers/hid/hid-lenovo-tpkbd.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c
index 762d988..31cf29a 100644
--- a/drivers/hid/hid-lenovo-tpkbd.c
+++ b/drivers/hid/hid-lenovo-tpkbd.c
@@ -414,22 +414,27 @@ static int tpkbd_probe(struct hid_device *hdev,
 	ret = hid_parse(hdev);
 	if (ret) {
 		hid_err(hdev, "hid_parse failed\n");
-		goto err_free;
+		goto err;
 	}
 
 	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 	if (ret) {
 		hid_err(hdev, "hid_hw_start failed\n");
-		goto err_free;
+		goto err;
 	}
 
 	uhdev = (struct usbhid_device *) hdev->driver_data;
 
-	if (uhdev->ifnum == 1)
-		return tpkbd_probe_tp(hdev);
+	if (uhdev->ifnum == 1) {
+		ret = tpkbd_probe_tp(hdev);
+		if (ret)
+			goto err_hid;
+	}
 
 	return 0;
-err_free:
+err_hid:
+	hid_hw_stop(hdev);
+err:
 	return ret;
 }
 
-- 
1.8.3.1

Re: [PATCH v3 06/10] HID: lenovo-tpkbd: validate output report details

[Kees Cook]

On Wed, Sep 11, 2013 at 12:56 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> From: Kees Cook <keescook@chromium.org>
>
> From: Kees Cook <keescook@chromium.org>
>

Not sure if "git am" will eat the second one, but the "From:" is
duplicated above...

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH v3 06/10] HID: lenovo-tpkbd: validate output report details

[Benjamin Tissoires]

On Wed, Sep 11, 2013 at 10:06 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Sep 11, 2013 at 12:56 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> From: Kees Cook <keescook@chromium.org>
>>
>> From: Kees Cook <keescook@chromium.org>
>>
>
> Not sure if "git am" will eat the second one, but the "From:" is
> duplicated above...
>

Yep, I saw that I duplicated it. Sorry :(
Anyway, feel free to add/remove any Signed-off-by if you wish (I think
I kept your original ones when it mattered).

Cheers,
Benjamin

Re: [PATCH v3 08/10] HID: validate feature and input report details

[Kees Cook]

On Wed, Sep 11, 2013 at 12:56 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> When dealing with usage_index, be sure to properly use unsigned instead of
> int to avoid overflows.
>
> When working on report fields, always validate that their report_counts are
> in bounds.
> Without this, a HID device could report a malicious feature report that
> could trick the driver into a heap overflow:
>
> [  634.885003] usb 1-1: New USB device found, idVendor=0596, idProduct=0500
> ...
> [  676.469629] BUG kmalloc-192 (Tainted: G        W   ): Redzone overwritten
>
> CVE-2013-2897
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

> ---
> v3:
>  - new patch: extract from the hid-multitouch patch, the generic checks so that
>    every hid drivers will benefit from them
>
>  drivers/hid/hid-core.c  | 16 +++++++---------
>  drivers/hid/hid-input.c | 11 ++++++++++-
>  2 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 44b6c68..329e24e 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -94,7 +94,6 @@ EXPORT_SYMBOL_GPL(hid_register_report);
>  static struct hid_field *hid_register_field(struct hid_report *report, unsigned usages, unsigned values)
>  {
>         struct hid_field *field;
> -       int i;
>
>         if (report->maxfield == HID_MAX_FIELDS) {
>                 hid_err(report->device, "too many fields in report\n");
> @@ -113,9 +112,6 @@ static struct hid_field *hid_register_field(struct hid_report *report, unsigned
>         field->value = (s32 *)(field->usage + usages);
>         field->report = report;
>
> -       for (i = 0; i < usages; i++)
> -               field->usage[i].usage_index = i;
> -
>         return field;
>  }
>
> @@ -226,9 +222,9 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
>  {
>         struct hid_report *report;
>         struct hid_field *field;
> -       int usages;
> +       unsigned usages;
>         unsigned offset;
> -       int i;
> +       unsigned i;
>
>         report = hid_register_report(parser->device, report_type, parser->global.report_id);
>         if (!report) {
> @@ -255,7 +251,8 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
>         if (!parser->local.usage_index) /* Ignore padding fields */
>                 return 0;
>
> -       usages = max_t(int, parser->local.usage_index, parser->global.report_count);
> +       usages = max_t(unsigned, parser->local.usage_index,
> +                                parser->global.report_count);
>
>         field = hid_register_field(report, usages, parser->global.report_count);
>         if (!field)
> @@ -266,13 +263,14 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
>         field->application = hid_lookup_collection(parser, HID_COLLECTION_APPLICATION);
>
>         for (i = 0; i < usages; i++) {
> -               int j = i;
> +               unsigned j = i;
>                 /* Duplicate the last usage we parsed if we have excess values */
>                 if (i >= parser->local.usage_index)
>                         j = parser->local.usage_index - 1;
>                 field->usage[i].hid = parser->local.usage[j];
>                 field->usage[i].collection_index =
>                         parser->local.collection_index[j];
> +               field->usage[i].usage_index = i;
>         }
>
>         field->maxusage = usages;
> @@ -1354,7 +1352,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
>                         goto out;
>         }
>
> -       if (hid->claimed != HID_CLAIMED_HIDRAW) {
> +       if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
>                 for (a = 0; a < report->maxfield; a++)
>                         hid_input_field(hid, report->field[a], cdata, interrupt);
>                 hdrv = hid->driver;
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index b420f4a..8741d95 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -485,6 +485,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>         if (field->flags & HID_MAIN_ITEM_CONSTANT)
>                 goto ignore;
>
> +       /* Ignore if report count is out of bounds. */
> +       if (field->report_count < 1)
> +               goto ignore;
> +
>         /* only LED usages are supported in output fields */
>         if (field->report_type == HID_OUTPUT_REPORT &&
>                         (usage->hid & HID_USAGE_PAGE) != HID_UP_LED) {
> @@ -1236,7 +1240,11 @@ static void report_features(struct hid_device *hid)
>
>         rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
>         list_for_each_entry(rep, &rep_enum->report_list, list)
> -               for (i = 0; i < rep->maxfield; i++)
> +               for (i = 0; i < rep->maxfield; i++) {
> +                       /* Ignore if report count is out of bounds. */
> +                       if (rep->field[i]->report_count < 1)
> +                               continue;
> +
>                         for (j = 0; j < rep->field[i]->maxusage; j++) {
>                                 /* Verify if Battery Strength feature is available */
>                                 hidinput_setup_battery(hid, HID_FEATURE_REPORT, rep->field[i]);
> @@ -1245,6 +1253,7 @@ static void report_features(struct hid_device *hid)
>                                         drv->feature_mapping(hid, rep->field[i],
>                                                              rep->field[i]->usage + j);
>                         }
> +               }
>  }
>
>  static struct hid_input *hidinput_allocate(struct hid_device *hid)
> --
> 1.8.3.1
>



-- 
Kees Cook
Chrome OS Security

Re: [PATCH v3 09/10] HID: multitouch: validate indexes details

[Kees Cook]

On Wed, Sep 11, 2013 at 12:56 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> When working on report indexes, always validate that they are in bounds.
> Without this, a HID device could report a malicious feature report that
> could trick the driver into a heap overflow:
>
> [  634.885003] usb 1-1: New USB device found, idVendor=0596, idProduct=0500
> ...
> [  676.469629] BUG kmalloc-192 (Tainted: G        W   ): Redzone overwritten
>
> Note that we need to change the indexes from s8 to s16 as they can
> be between -1 and 255.
>
> CVE-2013-2897
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

> ---
> v3:
>  - extract from hid-multitouch the generic checks so that every hid drivers will
>    benefit from them
>  - change __s8 index declarations into __s16
>  - use usage_index for the input_mode index instead of a half working code
>  - check the indexes validities only once
>
>  drivers/hid/hid-multitouch.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index ac28f08..5e5fe1b 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -101,9 +101,9 @@ struct mt_device {
>         unsigned last_slot_field;       /* the last field of a slot */
>         unsigned mt_report_id;  /* the report ID of the multitouch device */
>         unsigned pen_report_id; /* the report ID of the pen device */
> -       __s8 inputmode;         /* InputMode HID feature, -1 if non-existent */
> -       __s8 inputmode_index;   /* InputMode HID feature index in the report */
> -       __s8 maxcontact_report_id;      /* Maximum Contact Number HID feature,
> +       __s16 inputmode;        /* InputMode HID feature, -1 if non-existent */
> +       __s16 inputmode_index;  /* InputMode HID feature index in the report */
> +       __s16 maxcontact_report_id;     /* Maximum Contact Number HID feature,
>                                    -1 if non-existent */
>         __u8 num_received;      /* how many contacts we received */
>         __u8 num_expected;      /* expected last contact index */
> @@ -312,20 +312,18 @@ static void mt_feature_mapping(struct hid_device *hdev,
>                 struct hid_field *field, struct hid_usage *usage)
>  {
>         struct mt_device *td = hid_get_drvdata(hdev);
> -       int i;
>
>         switch (usage->hid) {
>         case HID_DG_INPUTMODE:
> -               td->inputmode = field->report->id;
> -               td->inputmode_index = 0; /* has to be updated below */
> -
> -               for (i=0; i < field->maxusage; i++) {
> -                       if (field->usage[i].hid == usage->hid) {
> -                               td->inputmode_index = i;
> -                               break;
> -                       }
> +               /* Ignore if value index is out of bounds. */
> +               if (usage->usage_index >= field->report_count) {
> +                       dev_err(&hdev->dev, "HID_DG_INPUTMODE out of range\n");
> +                       break;
>                 }
>
> +               td->inputmode = field->report->id;
> +               td->inputmode_index = usage->usage_index;
> +
>                 break;
>         case HID_DG_CONTACTMAX:
>                 td->maxcontact_report_id = field->report->id;
> @@ -511,6 +509,10 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>                         mt_store_field(usage, td, hi);
>                         return 1;
>                 case HID_DG_CONTACTCOUNT:
> +                       /* Ignore if indexes are out of bounds. */
> +                       if (field->index >= field->report->maxfield ||
> +                           usage->usage_index >= field->report_count)
> +                               return 1;
>                         td->cc_index = field->index;
>                         td->cc_value_index = usage->usage_index;
>                         return 1;
> --
> 1.8.3.1
>



-- 
Kees Cook
Chrome OS Security

Re: [PATCH v3 00/10] HID: validate report details

[Kees Cook]

On Wed, Sep 11, 2013 at 12:56 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Hi guys,
>
> here is the v3 of the CVE fixes.
>
> I have tested the multitouch and logitech-dj part, and the lenovo-tpkbd has been
> tested in the bug referenced in patch 10.
>
> Cheers,
> Benjamin
>
> Changes since v2:
> - fix lenovo-tpkbd report validation
> - fix lenovo-tpkbd not releasing the device when the report was not valid
> - use generic tests found in previous hid-multitouch patches, so that this will
>   not happen again
> - fix input_report index retrieving in hid-multitouch

Awesome! Thanks very much for fixing these up. I replied with Acks
where appropriate. :)

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH v3 03/10] HID: sony: validate HID output report details

[Josh Boyer]

On Wed, Sep 11, 2013 at 3:56 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> From: Kees Cook <keescook@chromium.org>
>
> This driver must validate the availability of the HID output report and
> its size before it can write LED states via buzz_set_leds(). This stops
> a heap overflow that is possible if a device provides a malicious HID
> output report:
>
> [  108.171280] usb 1-1: New USB device found, idVendor=054c, idProduct=0002
> ...
> [  117.507877] BUG kmalloc-192 (Not tainted): Redzone overwritten
>
> CVE-2013-2890
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@vger.kernel.org
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

As far as I know, this should be Cc: stable@vger.kernel.org #3.11, correct?

josh

> ---
> v3:
>  - no changes
>
>  drivers/hid/hid-sony.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 30dbb6b..b18320d 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -537,6 +537,10 @@ static int buzz_init(struct hid_device *hdev)
>         drv_data = hid_get_drvdata(hdev);
>         BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
>
> +       /* Validate expected report characteristics. */
> +       if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
> +               return -ENODEV;
> +
>         buzz = kzalloc(sizeof(*buzz), GFP_KERNEL);
>         if (!buzz) {
>                 hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 03/10] HID: sony: validate HID output report details

[Benjamin Tissoires]

On 12/09/13 14:39, Josh Boyer wrote:
> On Wed, Sep 11, 2013 at 3:56 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> From: Kees Cook <keescook@chromium.org>
>>
>> This driver must validate the availability of the HID output report and
>> its size before it can write LED states via buzz_set_leds(). This stops
>> a heap overflow that is possible if a device provides a malicious HID
>> output report:
>>
>> [  108.171280] usb 1-1: New USB device found, idVendor=054c, idProduct=0002
>> ...
>> [  117.507877] BUG kmalloc-192 (Not tainted): Redzone overwritten
>>
>> CVE-2013-2890
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Cc: stable@vger.kernel.org
>> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> As far as I know, this should be Cc: stable@vger.kernel.org #3.11, correct?
> 

Yep, for this one, only 3.11 is impacted.
Thanks Josh. And sorry for being to lazy to have added the proper tags :(

Cheers,
Benjamin

Re: [PATCH v3 00/10] HID: validate report details

[Jiri Kosina]

On Wed, 11 Sep 2013, Benjamin Tissoires wrote:

> here is the v3 of the CVE fixes.

Now applied, will be pushing to Linus for 3.12. Thanks everybody,

-- 
Jiri Kosina
SUSE Labs