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:09 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.