From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Kees Cook <keescook@chromium.org>,
Benjamin Tissoires <benjamin.tissoires@redhat.com>,
Jiri Kosina <jkosina@suse.cz>
Subject: [ 16/16] HID: LG: validate HID output report details
Date: Wed, 2 Oct 2013 21:05:40 -0700 [thread overview]
Message-ID: <20131003040446.646215809@linuxfoundation.org> (raw)
In-Reply-To: <20131003040445.523176877@linuxfoundation.org>
3.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Kees Cook <keescook@chromium.org>
commit 0fb6bd06e06792469acc15bbe427361b56ada528 upstream.
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@chromium.org>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/hid/hid-lg2ff.c | 19 +++----------------
drivers/hid/hid-lg3ff.c | 29 ++++++-----------------------
drivers/hid/hid-lg4ff.c | 20 +-------------------
drivers/hid/hid-lgff.c | 17 ++---------------
4 files changed, 12 insertions(+), 73 deletions(-)
--- a/drivers/hid/hid-lg2ff.c
+++ b/drivers/hid/hid-lg2ff.c
@@ -66,26 +66,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)) {
- hid_err(hid, "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) {
- hid_err(hid, "output report is empty\n");
- return -ENODEV;
- }
- if (report->field[0]->report_count < 7) {
- hid_err(hid, "not enough values in the field\n");
- return -ENODEV;
- }
lg2ff = kmalloc(sizeof(struct lg2ff_device), GFP_KERNEL);
if (!lg2ff)
--- a/drivers/hid/hid-lg3ff.c
+++ b/drivers/hid/hid-lg3ff.c
@@ -68,10 +68,11 @@ static int hid_lg3ff_play(struct input_d
int x, y;
/*
- * Maxusage should always be 63 (maximum fields)
- * likely a better way to ensure this data is clean
+ * Available values in the field should always be 63, but we only use up to
+ * 35. Instead, clear the entire area, however big it is.
*/
- memset(report->field[0]->value, 0, sizeof(__s32)*report->field[0]->maxusage);
+ memset(report->field[0]->value, 0,
+ sizeof(__s32) * report->field[0]->report_count);
switch (effect->type) {
case FF_CONSTANT:
@@ -131,32 +132,14 @@ static const signed short ff3_joystick_a
int lg3ff_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 = ff3_joystick_ac;
int error;
int i;
- /* Find the report to use */
- if (list_empty(report_list)) {
- hid_err(hid, "No output report found\n");
- return -1;
- }
-
/* Check that the report looks ok */
- report = list_entry(report_list->next, struct hid_report, list);
- if (!report) {
- hid_err(hid, "NULL output report\n");
- return -1;
- }
-
- field = report->field[0];
- if (!field) {
- hid_err(hid, "NULL field\n");
- return -1;
- }
+ if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 35))
+ return -ENODEV;
/* Assume single fixed device G940 */
for (i = 0; ff_bits[i] >= 0; i++)
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -339,33 +339,15 @@ static ssize_t lg4ff_range_store(struct
int lg4ff_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;
struct lg4ff_device_entry *entry;
struct usb_device_descriptor *udesc;
int error, i, j;
__u16 bcdDevice, rev_maj, rev_min;
- /* Find the report to use */
- if (list_empty(report_list)) {
- hid_err(hid, "No output report found\n");
- return -1;
- }
-
/* Check that the report looks ok */
- report = list_entry(report_list->next, struct hid_report, list);
- if (!report) {
- hid_err(hid, "NULL output report\n");
- return -1;
- }
-
- field = report->field[0];
- if (!field) {
- hid_err(hid, "NULL field\n");
+ if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7))
return -1;
- }
/* Check what wheel has been connected */
for (i = 0; i < ARRAY_SIZE(lg4ff_devices); i++) {
--- a/drivers/hid/hid-lgff.c
+++ b/drivers/hid/hid-lgff.c
@@ -130,27 +130,14 @@ static void hid_lgff_set_autocenter(stru
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)) {
- hid_err(hid, "No output report found\n");
- return -1;
- }
-
/* Check that the report looks ok */
- report = list_entry(report_list->next, struct hid_report, list);
- field = report->field[0];
- if (!field) {
- hid_err(hid, "NULL field\n");
- 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 &&
next prev parent reply other threads:[~2013-10-03 4:05 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-03 4:05 [ 00/16] 3.4.65-stable review Greg Kroah-Hartman
2013-10-03 4:05 ` [ 01/16] x86/reboot: Add quirk to make Dell C6100 use reboot=pci automatically Greg Kroah-Hartman
2013-10-03 4:05 ` [ 02/16] x86, efi: Dont map Boot Services on i386 Greg Kroah-Hartman
2013-10-03 4:05 ` [ 03/16] staging: vt6656: [BUG] main_usb.c oops on device_close move flag earlier Greg Kroah-Hartman
2013-10-03 4:05 ` [ 04/16] xhci: Ensure a command structure points to the correct trb on the command ring Greg Kroah-Hartman
2013-10-03 4:05 ` [ 05/16] xhci: Fix oops happening after address device timeout Greg Kroah-Hartman
2013-10-03 4:05 ` [ 06/16] xhci: Fix race between ep halt and URB cancellation Greg Kroah-Hartman
2013-10-03 4:05 ` [ 07/16] usb/core/devio.c: Dont reject control message to endpoint with wrong direction bit Greg Kroah-Hartman
2013-10-03 4:05 ` [ 08/16] dm snapshot: workaround for a false positive lockdep warning Greg Kroah-Hartman
2013-10-03 4:05 ` [ 09/16] dm-snapshot: fix performance degradation due to small hash size Greg Kroah-Hartman
2013-10-03 4:05 ` [ 10/16] drm/i915/dp: increase i2c-over-aux retry interval on AUX DEFER Greg Kroah-Hartman
2013-10-03 4:05 ` [ 11/16] drm/radeon: disable tests/benchmarks if accel is disabled Greg Kroah-Hartman
2013-10-03 4:05 ` [ 12/16] hwmon: (applesmc) Check key count before proceeding Greg Kroah-Hartman
2013-10-03 4:05 ` [ 13/16] ALSA: compress: Fix compress device unregister Greg Kroah-Hartman
2013-10-03 4:05 ` [ 14/16] mm, memcg: give exiting processes access to memory reserves Greg Kroah-Hartman
2013-10-03 4:05 ` [ 15/16] mm: fix aio performance regression for database caused by THP Greg Kroah-Hartman
2013-10-03 4:05 ` Greg Kroah-Hartman [this message]
2013-10-03 13:30 ` [ 00/16] 3.4.65-stable review Guenter Roeck
2013-10-03 18:40 ` Greg Kroah-Hartman
2013-10-03 21:19 ` Guenter Roeck
2013-10-03 18:39 ` Greg Kroah-Hartman
2013-10-04 0:24 ` Shuah Khan
2013-10-04 2:38 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131003040446.646215809@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=benjamin.tissoires@redhat.com \
--cc=jkosina@suse.cz \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).