[PATCH v2] hid: hid-sensor-hub: clear memory to avoid random data

[PATCH v2] hid: hid-sensor-hub: clear memory to avoid random data

[Song Hongyan]

Initialize user buffer with 0s. This will avoid random data in the buffer,
when the user buffer size is bigger than the actual report size.

Signed-off-by: Song Hongyan <hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
Changes in v2:
    -clear memory in get_feature() instead of set_feature()

 drivers/hid/hid-sensor-hub.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 658a607..b74d954 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -252,6 +252,7 @@ int sensor_hub_get_feature(struct hid_sensor_hub_device *hsdev, u32 report_id,
 	int report_size;
 	int ret = 0;
 
+	memset(buffer, 0, buffer_size);
 	mutex_lock(&data->mutex);
 	report = sensor_hub_report(report_id, hsdev->hdev, HID_FEATURE_REPORT);
 	if (!report || (field_index >= report->maxfield) ||
-- 
1.9.1

Re: [PATCH v2] hid: hid-sensor-hub: clear memory to avoid random data

[Jonathan Cameron]

On 11/11/16 00:58, Song Hongyan wrote:
> Initialize user buffer with 0s. This will avoid random data in the buffer,
> when the user buffer size is bigger than the actual report size.
> 
> Signed-off-by: Song Hongyan <hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Please describe the result of this bug in the description.
Why does it matter?

Without that info, it's hard to judge what path this should take
into mainline or whether the fix needs to be marked for stable.

Thanks,

Jonathan
> ---
> Changes in v2:
>     -clear memory in get_feature() instead of set_feature()
> 
>  drivers/hid/hid-sensor-hub.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 658a607..b74d954 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -252,6 +252,7 @@ int sensor_hub_get_feature(struct hid_sensor_hub_device *hsdev, u32 report_id,
>  	int report_size;
>  	int ret = 0;
>  
> +	memset(buffer, 0, buffer_size);
>  	mutex_lock(&data->mutex);
>  	report = sensor_hub_report(report_id, hsdev->hdev, HID_FEATURE_REPORT);
>  	if (!report || (field_index >= report->maxfield) ||
> 

Re: [PATCH v2] hid: hid-sensor-hub: clear memory to avoid random data

[Pandruvada, Srinivas]

On Sat, 2016-11-12 at 14:43 +0000, Jonathan Cameron wrote:
> On 11/11/16 00:58, Song Hongyan wrote:
> > 
> > Initialize user buffer with 0s. This will avoid random data in the
> > buffer,
> > when the user buffer size is bigger than the actual report size.
> > 
> > Signed-off-by: Song Hongyan <hongyan.song@intel.com>
> Please describe the result of this bug in the description.
> Why does it matter?
Hongyan,

Please resubmit.



The issue is:

When user tried to read some fields like hysteresis from IIO sysfs on
some systems, it fails. The reason is that this field is a byte field
and caller of sensor_hub_get_feature() passes a buffer of 4 bytes. Here
the function sensor_hub_get_feature() copies the single byte from the
report to the caller buffer and returns "1" as the number of bytes
copied. So caller can use the return value.

But this is done by multiple callers, so if we just change the
sensor_hub_get_feature so that caller buffer is initialized with 0s
then we don't to change all functions. 



> 
> Without that info, it's hard to judge what path this should take
> into mainline or whether the fix needs to be marked for stable.
IMO this is not an urgent fix and queued for next kernel release.


> Thanks,
> 
> Jonathan
> > 
> > ---
> > Changes in v2:
> >     -clear memory in get_feature() instead of set_feature()
s/get_feature()/sensor_hub_get_feature()


Thanks,
Srinivas

> > 
> >  drivers/hid/hid-sensor-hub.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> > hub.c
> > index 658a607..b74d954 100644
> > --- a/drivers/hid/hid-sensor-hub.c
> > +++ b/drivers/hid/hid-sensor-hub.c
> > @@ -252,6 +252,7 @@ int sensor_hub_get_feature(struct
> > hid_sensor_hub_device *hsdev, u32 report_id,
> >  	int report_size;
> >  	int ret = 0;
> >  
> > +	memset(buffer, 0, buffer_size);
> >  	mutex_lock(&data->mutex);
> >  	report = sensor_hub_report(report_id, hsdev->hdev,
> > HID_FEATURE_REPORT);
> >  	if (!report || (field_index >= report->maxfield) ||
> >