Re: [PATCH 2/7] iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unsafe memory access via out-of-bounds read and unaligned dereference.
--

commit f0677ef9176d20b908a2aa7a22d8817abe7925a7
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>

iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned

This commit updates the hid-sensor-accel-3d driver to use u32 instead of
unsigned for the usage_id variables. This improves code clarity and maintains
consistency with kernel coding styles.

> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c

[ ... ]

> @@ -247,7 +247,7 @@ static int accel_3d_proc_event(struct hid_sensor_hub_device *hsdev,
>  
>  /* Capture samples in local storage */
>  static int accel_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> -				unsigned usage_id,
> +				u32 usage_id,
>  				size_t raw_len, char *raw_data,
>  				void *priv)
>  {
>  	struct iio_dev *indio_dev = platform_get_drvdata(priv);
>  	struct accel_3d_state *accel_state = iio_priv(indio_dev);
>  	int offset;
>  	int ret = -EINVAL;
>  
>  	switch (usage_id) {
>  	case HID_USAGE_SENSOR_ACCEL_X_AXIS:
>  	case HID_USAGE_SENSOR_ACCEL_Y_AXIS:
>  	case HID_USAGE_SENSOR_ACCEL_Z_AXIS:
>  		offset = usage_id - HID_USAGE_SENSOR_ACCEL_X_AXIS;
>  		accel_state->scan.accel_val[CHANNEL_SCAN_INDEX_X + offset] =
>  						*(u32 *)raw_data;

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem with how
raw_data is accessed here. Does this direct cast to a 32-bit integer pointer
assume that raw_len is always at least 4 bytes and that raw_data is always
4-byte aligned?

If a device provides a smaller report, could this result in an out-of-bounds
read? Also, since raw_data comes from a HID report buffer, could this trigger
an alignment fault on architectures with strict alignment requirements if the
data is unaligned?

>  		ret = 0;
>  	break;
>  	case HID_USAGE_SENSOR_TIME_TIMESTAMP:
>  		accel_state->timestamp =
>  			hid_sensor_convert_timestamp(
>  					&accel_state->common_attributes,
>  					*(int64_t *)raw_data);

[Severity: High]
This is also a pre-existing issue, but similar to the above, does this direct
cast to a 64-bit integer pointer require raw_len to be at least 8 bytes and
raw_data to be 8-byte aligned?

Would it be safer to use get_unaligned() and check raw_len before
dereferencing these pointers to avoid potential slab-out-of-bounds panics
or alignment faults?

>  		ret = 0;
>  	break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com?part=2