Re: [PATCH v2 RESEND] HID: sensor-hub: Fix out-of-bounds write in sensor_hub_get_feature

On Wed, 2026-08-05 at 18:57 +0000, bakabaka9 wrote:
> From: Xingrui Li <[email protected]>
> 
> sensor_hub_get_feature() clamps its return value to the caller's
> buffer
> size, but the copy loop still copies field->report_size / 8 bytes for
> each report value. A malicious HID descriptor can advertise a large
> feature field size while an IIO caller supplies a small stack buffer,
> such as a single s32, causing an out-of-bounds write.
> 
> HID core stores parsed report values in __s32 slots and clamps
> extracted
> values to 32 bits. Reject feature fields that require more than one
> slot
> per value, guard the total byte count calculation, and clamp each
> per-value copy to the remaining caller buffer.
> 
> Fixes: 5459ada2b3cd69 ("HID: sensor-hub: Fix packing of result buffer
> for feature report")
> Cc: [email protected]
> Assisted-by: OpenAI:GPT-5.5-Cyber
> Signed-off-by: Xingrui Li <[email protected]>

Acked-by: Srinivas Pandruvada <[email protected]>

> ---
>  drivers/hid/hid-sensor-hub.c | 42 +++++++++++++++++++++-------------
> --
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> hub.c
> index 34f710c465b8..6470a290ebfc 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -239,12 +239,17 @@ int sensor_hub_get_feature(struct
> hid_sensor_hub_device *hsdev, u32 report_id,
>  			   u32 field_index, int buffer_size, void
> *buffer)
>  {
>  	struct hid_report *report;
> +	struct hid_field *field;
>  	struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
> -	int report_size;
> +	size_t field_size;
> +	size_t report_size;
> +	size_t copied = 0;
> +	size_t to_copy;
>  	int ret = 0;
> -	u8 *val_ptr;
> -	int buffer_index = 0;
> -	int i;
> +	unsigned int i;
> +
> +	if (!buffer || buffer_size <= 0)
> +		return -EINVAL;
>  
>  	memset(buffer, 0, buffer_size);
>  
> @@ -258,26 +263,29 @@ int sensor_hub_get_feature(struct
> hid_sensor_hub_device *hsdev, u32 report_id,
>  	hid_hw_request(hsdev->hdev, report, HID_REQ_GET_REPORT);
>  	hid_hw_wait(hsdev->hdev);
>  
> +	field = report->field[field_index];
> +
>  	/* calculate number of bytes required to read this field */
> -	report_size = DIV_ROUND_UP(report->field[field_index]-
> >report_size,
> -				   8) *
> -				   report->field[field_index]-
> >report_count;
> -	if (!report_size) {
> +	field_size = DIV_ROUND_UP(field->report_size, 8);
> +	/* HID core stores each parsed report value in a __s32 slot.
> */
> +	if (!field_size || field_size > sizeof(field->value[0])) {
> +		ret = -EINVAL;
> +		goto done_proc;
> +	}
> +	if (field->report_count > SIZE_MAX / field_size) {
>  		ret = -EINVAL;
>  		goto done_proc;
>  	}
> -	ret = min(report_size, buffer_size);
>  
> -	val_ptr = (u8 *)report->field[field_index]->value;
> -	for (i = 0; i < report->field[field_index]->report_count;
> ++i) {
> -		if (buffer_index >= ret)
> -			break;
> +	report_size = field_size * field->report_count;
> +	report_size = min_t(size_t, report_size, buffer_size);
>  
> -		memcpy(&((u8 *)buffer)[buffer_index], val_ptr,
> -		       report->field[field_index]->report_size / 8);
> -		val_ptr += sizeof(__s32);
> -		buffer_index += (report->field[field_index]-
> >report_size / 8);
> +	for (i = 0; i < field->report_count && copied < report_size;
> ++i) {
> +		to_copy = min(field_size, report_size - copied);
> +		memcpy(&((u8 *)buffer)[copied], &field->value[i],
> to_copy);
> +		copied += to_copy;
>  	}
> +	ret = copied;
>  
>  done_proc:
>  	mutex_unlock(&data->mutex);