On 08/28/2013 02:16 PM, Kees Cook wrote:

On Wed, Aug 28, 2013 at 1:42 PM, Jiri Kosina [off-list ref] wrote:

quoted

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.

Agree fix is simple, but the malicious feature report can contains other 
junk also. Can we really address all such issues?

quoted

quoted

quoted

             ret = -EINVAL;
             goto done_proc;
     }

Thanks,
Srinivas

--
Jiri Kosina
SUSE Labs

Thanks,

Srinivas