[PATCH 3.2 58/61] Input: gtco - fix potential out-of-bound access

From: Ben Hutchings
Date: Tue Nov 21 2017 - 22:07:17 EST


3.2.96-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>

commit a50829479f58416a013a4ccca791336af3c584c7 upstream.

parse_hid_report_descriptor() has a while (i < length) loop, which
only guarantees that there's at least 1 byte in the buffer, but the
loop body can read multiple bytes which causes out-of-bounds access.

Reported-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
Reviewed-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
[bwh: Backported to 3.2: use &device->usbdev->dev as the device for dev_err()]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
drivers/input/tablet/gtco.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

--- a/drivers/input/tablet/gtco.c
+++ b/drivers/input/tablet/gtco.c
@@ -232,13 +232,17 @@ static void parse_hid_report_descriptor(

/* Walk this report and pull out the info we need */
while (i < length) {
- prefix = report[i];
-
- /* Skip over prefix */
- i++;
+ prefix = report[i++];

/* Determine data size and save the data in the proper variable */
- size = PREF_SIZE(prefix);
+ size = (1U << PREF_SIZE(prefix)) >> 1;
+ if (i + size > length) {
+ dev_err(&device->usbdev->dev,
+ "Not enough data (need %d, have %d)\n",
+ i + size, length);
+ break;
+ }
+
switch (size) {
case 1:
data = report[i];
@@ -246,8 +250,7 @@ static void parse_hid_report_descriptor(
case 2:
data16 = get_unaligned_le16(&report[i]);
break;
- case 3:
- size = 4;
+ case 4:
data32 = get_unaligned_le32(&report[i]);
break;
}