Skip to content

Commit 0fb6bd0

Browse files
keesJiri Kosina
authored andcommitted
HID: LG: validate HID output report details
A HID device could send a malicious output report that would cause the lg, lg3, and lg4 HID drivers to write beyond the output report allocation during an event, causing a heap overflow: [ 325.245240] usb 1-1: New USB device found, idVendor=046d, idProduct=c287 ... [ 414.518960] BUG kmalloc-4096 (Not tainted): Redzone overwritten Additionally, while lg2 did correctly validate the report details, it was cleaned up and shortened. CVE-2013-2893 Signed-off-by: Kees Cook <[email protected]> Cc: [email protected] Reviewed-by: Benjamin Tissoires <[email protected]> Signed-off-by: Jiri Kosina <[email protected]>
1 parent 41df7f6 commit 0fb6bd0

File tree

4 files changed

+12
-73
lines changed

4 files changed

+12
-73
lines changed

drivers/hid/hid-lg2ff.c

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,26 +64,13 @@ int lg2ff_init(struct hid_device *hid)
6464
struct hid_report *report;
6565
struct hid_input *hidinput = list_entry(hid->inputs.next,
6666
struct hid_input, list);
67-
struct list_head *report_list =
68-
&hid->report_enum[HID_OUTPUT_REPORT].report_list;
6967
struct input_dev *dev = hidinput->input;
7068
int error;
7169

72-
if (list_empty(report_list)) {
73-
hid_err(hid, "no output report found\n");
70+
/* Check that the report looks ok */
71+
report = hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7);
72+
if (!report)
7473
return -ENODEV;
75-
}
76-
77-
report = list_entry(report_list->next, struct hid_report, list);
78-
79-
if (report->maxfield < 1) {
80-
hid_err(hid, "output report is empty\n");
81-
return -ENODEV;
82-
}
83-
if (report->field[0]->report_count < 7) {
84-
hid_err(hid, "not enough values in the field\n");
85-
return -ENODEV;
86-
}
8774

8875
lg2ff = kmalloc(sizeof(struct lg2ff_device), GFP_KERNEL);
8976
if (!lg2ff)

drivers/hid/hid-lg3ff.c

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,11 @@ static int hid_lg3ff_play(struct input_dev *dev, void *data,
6666
int x, y;
6767

6868
/*
69-
* Maxusage should always be 63 (maximum fields)
70-
* likely a better way to ensure this data is clean
69+
* Available values in the field should always be 63, but we only use up to
70+
* 35. Instead, clear the entire area, however big it is.
7171
*/
72-
memset(report->field[0]->value, 0, sizeof(__s32)*report->field[0]->maxusage);
72+
memset(report->field[0]->value, 0,
73+
sizeof(__s32) * report->field[0]->report_count);
7374

7475
switch (effect->type) {
7576
case FF_CONSTANT:
@@ -129,32 +130,14 @@ static const signed short ff3_joystick_ac[] = {
129130
int lg3ff_init(struct hid_device *hid)
130131
{
131132
struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
132-
struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
133133
struct input_dev *dev = hidinput->input;
134-
struct hid_report *report;
135-
struct hid_field *field;
136134
const signed short *ff_bits = ff3_joystick_ac;
137135
int error;
138136
int i;
139137

140-
/* Find the report to use */
141-
if (list_empty(report_list)) {
142-
hid_err(hid, "No output report found\n");
143-
return -1;
144-
}
145-
146138
/* Check that the report looks ok */
147-
report = list_entry(report_list->next, struct hid_report, list);
148-
if (!report) {
149-
hid_err(hid, "NULL output report\n");
150-
return -1;
151-
}
152-
153-
field = report->field[0];
154-
if (!field) {
155-
hid_err(hid, "NULL field\n");
156-
return -1;
157-
}
139+
if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 35))
140+
return -ENODEV;
158141

159142
/* Assume single fixed device G940 */
160143
for (i = 0; ff_bits[i] >= 0; i++)

drivers/hid/hid-lg4ff.c

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -484,34 +484,16 @@ static enum led_brightness lg4ff_led_get_brightness(struct led_classdev *led_cde
484484
int lg4ff_init(struct hid_device *hid)
485485
{
486486
struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
487-
struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
488487
struct input_dev *dev = hidinput->input;
489-
struct hid_report *report;
490-
struct hid_field *field;
491488
struct lg4ff_device_entry *entry;
492489
struct lg_drv_data *drv_data;
493490
struct usb_device_descriptor *udesc;
494491
int error, i, j;
495492
__u16 bcdDevice, rev_maj, rev_min;
496493

497-
/* Find the report to use */
498-
if (list_empty(report_list)) {
499-
hid_err(hid, "No output report found\n");
500-
return -1;
501-
}
502-
503494
/* Check that the report looks ok */
504-
report = list_entry(report_list->next, struct hid_report, list);
505-
if (!report) {
506-
hid_err(hid, "NULL output report\n");
495+
if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7))
507496
return -1;
508-
}
509-
510-
field = report->field[0];
511-
if (!field) {
512-
hid_err(hid, "NULL field\n");
513-
return -1;
514-
}
515497

516498
/* Check what wheel has been connected */
517499
for (i = 0; i < ARRAY_SIZE(lg4ff_devices); i++) {

drivers/hid/hid-lgff.c

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -128,27 +128,14 @@ static void hid_lgff_set_autocenter(struct input_dev *dev, u16 magnitude)
128128
int lgff_init(struct hid_device* hid)
129129
{
130130
struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
131-
struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
132131
struct input_dev *dev = hidinput->input;
133-
struct hid_report *report;
134-
struct hid_field *field;
135132
const signed short *ff_bits = ff_joystick;
136133
int error;
137134
int i;
138135

139-
/* Find the report to use */
140-
if (list_empty(report_list)) {
141-
hid_err(hid, "No output report found\n");
142-
return -1;
143-
}
144-
145136
/* Check that the report looks ok */
146-
report = list_entry(report_list->next, struct hid_report, list);
147-
field = report->field[0];
148-
if (!field) {
149-
hid_err(hid, "NULL field\n");
150-
return -1;
151-
}
137+
if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7))
138+
return -ENODEV;
152139

153140
for (i = 0; i < ARRAY_SIZE(devices); i++) {
154141
if (dev->id.vendor == devices[i].idVendor &&

0 commit comments

Comments
 (0)