3.4-stable review patch. If anyone has any objections, please let me know.
------------------ From: Benjamin Tissoires <[email protected]> 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 Cc: [email protected] Signed-off-by: Benjamin Tissoires <[email protected]> Acked-by: Kees Cook <[email protected]> Signed-off-by: Jiri Kosina <[email protected]> [bwh: Backported to 3.2: - Drop inapplicable changes to hid_usage::usage_index initialisation and to hid_report_raw_event() - Adjust context in report_features() Signed-off-by: Ben Hutchings <[email protected]> [yijingwang: Backported to 3.4: context adjust] Signed-off-by: Yijing Wang <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> --- drivers/hid/hid-core.c | 9 +++++---- drivers/hid/hid-input.c | 12 +++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-) --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -222,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) { @@ -244,7 +244,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) @@ -255,7 +256,7 @@ 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; --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -459,6 +459,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) { @@ -1119,7 +1123,12 @@ 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]); @@ -1128,6 +1137,7 @@ static void report_features(struct hid_d drv->feature_mapping(hid, rep->field[i], rep->field[i]->usage + j); } + } } /* -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
