Re: [PATCH 12/14] HID: sensor-hub: validate feature report details
From: Kees Cook <hidden>
Date: 2013-08-28 21:16:43
On Wed, Aug 28, 2013 at 1:42 PM, Jiri Kosina [off-list ref] wrote:
On Wed, 28 Aug 2013, Srinivas Pandruvada wrote:quoted
quoted
A HID device could send a malicious feature report that would cause the sensor-hub HID driver to read past the end of heap allocation, leaking kernel memory contents to the caller. CVE-2013-2898 Signed-off-by: Kees Cook <redacted> Cc: stable@kernel.org --- drivers/hid/hid-sensor-hub.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c index ca749810..aa34755 100644 --- a/drivers/hid/hid-sensor-hub.c +++ b/drivers/hid/hid-sensor-hub.c@@ -221,7 +221,8 @@ int sensor_hub_get_feature(struct hid_sensor_hub_device*hsdev, u32 report_id, mutex_lock(&data->mutex); report = sensor_hub_report(report_id, hsdev->hdev, HID_FEATURE_REPORT); - if (!report || (field_index >= report->maxfield)) { + if (!report || (field_index >= report->maxfield) || + report->field[field_index]->report_count < 1) {Is it based on some HID device is sending junk report or just from a code review?My understanding is that this whole Kees' patchset is about potentially evil devices doing bad things (on purpose).
Correct, though this particular flaw is pretty weak. It requires both malicious device and malicious user-space. However, with the advent of things like HTML5 USB API, it's possible these could be combined to attack a device. Regardless, this fix seems obviously correct and trivial to me. -Kees
quoted
quoted
ret = -EINVAL; goto done_proc; }Thanks, Srinivas-- Jiri Kosina SUSE Labs
-- Kees Cook Chrome OS Security