Skip to content

Commit cc6b54a

Browse files
bentissJiri Kosina
authored andcommitted
HID: validate feature and input report details
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: [email protected] Signed-off-by: Benjamin Tissoires <[email protected]> Acked-by: Kees Cook <[email protected]> Signed-off-by: Jiri Kosina <[email protected]>
1 parent 0a9cd0a commit cc6b54a

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

drivers/hid/hid-core.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ EXPORT_SYMBOL_GPL(hid_register_report);
9494
static struct hid_field *hid_register_field(struct hid_report *report, unsigned usages, unsigned values)
9595
{
9696
struct hid_field *field;
97-
int i;
9897

9998
if (report->maxfield == HID_MAX_FIELDS) {
10099
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
113112
field->value = (s32 *)(field->usage + usages);
114113
field->report = report;
115114

116-
for (i = 0; i < usages; i++)
117-
field->usage[i].usage_index = i;
118-
119115
return field;
120116
}
121117

@@ -226,9 +222,9 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
226222
{
227223
struct hid_report *report;
228224
struct hid_field *field;
229-
int usages;
225+
unsigned usages;
230226
unsigned offset;
231-
int i;
227+
unsigned i;
232228

233229
report = hid_register_report(parser->device, report_type, parser->global.report_id);
234230
if (!report) {
@@ -255,7 +251,8 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
255251
if (!parser->local.usage_index) /* Ignore padding fields */
256252
return 0;
257253

258-
usages = max_t(int, parser->local.usage_index, parser->global.report_count);
254+
usages = max_t(unsigned, parser->local.usage_index,
255+
parser->global.report_count);
259256

260257
field = hid_register_field(report, usages, parser->global.report_count);
261258
if (!field)
@@ -266,13 +263,14 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
266263
field->application = hid_lookup_collection(parser, HID_COLLECTION_APPLICATION);
267264

268265
for (i = 0; i < usages; i++) {
269-
int j = i;
266+
unsigned j = i;
270267
/* Duplicate the last usage we parsed if we have excess values */
271268
if (i >= parser->local.usage_index)
272269
j = parser->local.usage_index - 1;
273270
field->usage[i].hid = parser->local.usage[j];
274271
field->usage[i].collection_index =
275272
parser->local.collection_index[j];
273+
field->usage[i].usage_index = i;
276274
}
277275

278276
field->maxusage = usages;
@@ -1354,7 +1352,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
13541352
goto out;
13551353
}
13561354

1357-
if (hid->claimed != HID_CLAIMED_HIDRAW) {
1355+
if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
13581356
for (a = 0; a < report->maxfield; a++)
13591357
hid_input_field(hid, report->field[a], cdata, interrupt);
13601358
hdrv = hid->driver;

drivers/hid/hid-input.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
485485
if (field->flags & HID_MAIN_ITEM_CONSTANT)
486486
goto ignore;
487487

488+
/* Ignore if report count is out of bounds. */
489+
if (field->report_count < 1)
490+
goto ignore;
491+
488492
/* only LED usages are supported in output fields */
489493
if (field->report_type == HID_OUTPUT_REPORT &&
490494
(usage->hid & HID_USAGE_PAGE) != HID_UP_LED) {
@@ -1236,7 +1240,11 @@ static void report_features(struct hid_device *hid)
12361240

12371241
rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
12381242
list_for_each_entry(rep, &rep_enum->report_list, list)
1239-
for (i = 0; i < rep->maxfield; i++)
1243+
for (i = 0; i < rep->maxfield; i++) {
1244+
/* Ignore if report count is out of bounds. */
1245+
if (rep->field[i]->report_count < 1)
1246+
continue;
1247+
12401248
for (j = 0; j < rep->field[i]->maxusage; j++) {
12411249
/* Verify if Battery Strength feature is available */
12421250
hidinput_setup_battery(hid, HID_FEATURE_REPORT, rep->field[i]);
@@ -1245,6 +1253,7 @@ static void report_features(struct hid_device *hid)
12451253
drv->feature_mapping(hid, rep->field[i],
12461254
rep->field[i]->usage + j);
12471255
}
1256+
}
12481257
}
12491258

12501259
static struct hid_input *hidinput_allocate(struct hid_device *hid)

0 commit comments

Comments
 (0)