linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Benjamin Tissoires <benjamin.tissoires@gmail.com>,
	Kees Cook <keescook@chromium.org>,
	Henrik Rydberg <rydberg@euromail.se>,
	Jiri Kosina <jkosina@suse.cz>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v3 09/10] HID: multitouch: validate indexes details
Date: Wed, 11 Sep 2013 21:56:58 +0200	[thread overview]
Message-ID: <1378929419-6269-10-git-send-email-benjamin.tissoires@redhat.com> (raw)
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>

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

  parent reply	other threads:[~2013-09-11 19:56 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-11 19:56 [PATCH v3 00/10] HID: validate report details Benjamin Tissoires
2013-09-11 19:56 ` [PATCH v3 01/10] HID: provide a helper for validating hid reports Benjamin Tissoires
2013-09-11 19:56 ` [PATCH v3 02/10] HID: zeroplus: validate output report details Benjamin Tissoires
2013-09-11 19:56 ` [PATCH v3 03/10] HID: sony: validate HID " Benjamin Tissoires
2013-09-12 12:39   ` Josh Boyer
2013-09-12 14:11     ` Benjamin Tissoires
2013-09-11 19:56 ` [PATCH v3 04/10] HID: steelseries: validate " Benjamin Tissoires
2013-09-11 19:56 ` [PATCH v3 05/10] HID: LG: validate HID " Benjamin Tissoires
2013-09-11 19:56 ` [PATCH v3 06/10] HID: lenovo-tpkbd: validate " Benjamin Tissoires
2013-09-11 20:06   ` Kees Cook
2013-09-11 20:08     ` Benjamin Tissoires
2013-09-11 19:56 ` [PATCH v3 07/10] HID: logitech-dj: " Benjamin Tissoires
2013-09-11 19:56 ` [PATCH v3 08/10] HID: validate feature and input " Benjamin Tissoires
2013-09-11 20:08   ` Kees Cook
2013-09-11 19:56 ` Benjamin Tissoires [this message]
2013-09-11 20:09   ` [PATCH v3 09/10] HID: multitouch: validate indexes details Kees Cook
2013-09-11 19:56 ` [PATCH v3 10/10] HID: lenovo-tpkbd: fix leak if tpkbd_probe_tp fails Benjamin Tissoires
2013-09-11 20:11 ` [PATCH v3 00/10] HID: validate report details Kees Cook
2013-09-13 13:15 ` Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1378929419-6269-10-git-send-email-benjamin.tissoires@redhat.com \
    --to=benjamin.tissoires@redhat.com \
    --cc=benjamin.tissoires@gmail.com \
    --cc=jkosina@suse.cz \
    --cc=keescook@chromium.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rydberg@euromail.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).