| From cc6b54aa54bf40b762cab45a9fc8aa81653146eb Mon Sep 17 00:00:00 2001 |
| From: Benjamin Tissoires <benjamin.tissoires@redhat.com> |
| Date: Wed, 11 Sep 2013 21:56:57 +0200 |
| Subject: HID: validate feature and input report details |
| |
| From: Benjamin Tissoires <benjamin.tissoires@redhat.com> |
| |
| commit cc6b54aa54bf40b762cab45a9fc8aa81653146eb upstream. |
| |
| 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 |
| |
| Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> |
| Acked-by: Kees Cook <keescook@chromium.org> |
| Signed-off-by: Jiri Kosina <jkosina@suse.cz> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/hid/hid-core.c | 16 +++++++--------- |
| drivers/hid/hid-input.c | 11 ++++++++++- |
| 2 files changed, 17 insertions(+), 10 deletions(-) |
| |
| --- 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_fi |
| 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_pars |
| { |
| 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_pars |
| 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_pars |
| 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; |
| @@ -1295,7 +1293,7 @@ int hid_report_raw_event(struct hid_devi |
| 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; |
| --- a/drivers/hid/hid-input.c |
| +++ b/drivers/hid/hid-input.c |
| @@ -485,6 +485,10 @@ static void hidinput_configure_usage(str |
| 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) { |
| @@ -1168,7 +1172,11 @@ static void report_features(struct hid_d |
| |
| 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]); |
| @@ -1177,6 +1185,7 @@ static void report_features(struct hid_d |
| drv->feature_mapping(hid, rep->field[i], |
| rep->field[i]->usage + j); |
| } |
| + } |
| } |
| |
| static struct hid_input *hidinput_allocate(struct hid_device *hid) |