Ack: [Lucid][CVE-2013-2893] HID: LG: validate HID output report details

[Brad Figg]

On 10/03/2013 09:08 AM, Luis Henriques wrote:
> From: Kees Cook <keescook at chromium.org>
> 
> BugLink: http://bugs.launchpad.net/bugs/1220190
> 
> CVE-2013-2893
> 
> 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 <keescook at chromium.org>
> Cc: stable at vger.kernel.org
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires at redhat.com>
> Signed-off-by: Jiri Kosina <jkosina at suse.cz>
> (back ported from commit 0fb6bd06e06792469acc15bbe427361b56ada528)
> [ luis: dropped changes to files drivers/hid/hid-lg3ff.c and
>   drivers/hid/hid-lg4ff.c ]
> Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> ---
>  drivers/hid/hid-lg2ff.c | 19 +++----------------
>  drivers/hid/hid-lgff.c  | 17 ++---------------
>  2 files changed, 5 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/hid/hid-lg2ff.c b/drivers/hid/hid-lg2ff.c
> index 4e6dc6e..a260a8c 100644
> --- a/drivers/hid/hid-lg2ff.c
> +++ b/drivers/hid/hid-lg2ff.c
> @@ -65,26 +65,13 @@ int lg2ff_init(struct hid_device *hid)
>  	struct hid_report *report;
>  	struct hid_input *hidinput = list_entry(hid->inputs.next,
>  						struct hid_input, list);
> -	struct list_head *report_list =
> -			&hid->report_enum[HID_OUTPUT_REPORT].report_list;
>  	struct input_dev *dev = hidinput->input;
>  	int error;
>  
> -	if (list_empty(report_list)) {
> -		dev_err(&hid->dev, "no output report found\n");
> +	/* Check that the report looks ok */
> +	report = hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7);
> +	if (!report)
>  		return -ENODEV;
> -	}
> -
> -	report = list_entry(report_list->next, struct hid_report, list);
> -
> -	if (report->maxfield < 1) {
> -		dev_err(&hid->dev, "output report is empty\n");
> -		return -ENODEV;
> -	}
> -	if (report->field[0]->report_count < 7) {
> -		dev_err(&hid->dev, "not enough values in the field\n");
> -		return -ENODEV;
> -	}
>  
>  	lg2ff = kmalloc(sizeof(struct lg2ff_device), GFP_KERNEL);
>  	if (!lg2ff)
> diff --git a/drivers/hid/hid-lgff.c b/drivers/hid/hid-lgff.c
> index 987abeb..df26abb 100644
> --- a/drivers/hid/hid-lgff.c
> +++ b/drivers/hid/hid-lgff.c
> @@ -135,27 +135,14 @@ static void hid_lgff_set_autocenter(struct input_dev *dev, u16 magnitude)
>  int lgff_init(struct hid_device* hid)
>  {
>  	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
> -	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
>  	struct input_dev *dev = hidinput->input;
> -	struct hid_report *report;
> -	struct hid_field *field;
>  	const signed short *ff_bits = ff_joystick;
>  	int error;
>  	int i;
>  
> -	/* Find the report to use */
> -	if (list_empty(report_list)) {
> -		err_hid("No output report found");
> -		return -1;
> -	}
> -
>  	/* Check that the report looks ok */
> -	report = list_entry(report_list->next, struct hid_report, list);
> -	field = report->field[0];
> -	if (!field) {
> -		err_hid("NULL field");
> -		return -1;
> -	}
> +	if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7))
> +		return -ENODEV;
>  
>  	for (i = 0; i < ARRAY_SIZE(devices); i++) {
>  		if (dev->id.vendor == devices[i].idVendor &&
> 


-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com