Re: [PATCH 7/7] iio: pressure: hid-sensor-press: use u32 instead of unsigned

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unaligned memory access and out-of-bounds read in `press_capture_sample` due to unconditional pointer cast and ignoring `raw_len`.
- [High] Stale software timestamp reused for all subsequent samples if the device lacks hardware timestamps.
--

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

iio: pressure: hid-sensor-press: use u32 instead of unsigned

Prefer 'u32' instead of bare 'unsigned' variable to improve code
clarity and consistency with kernel style.

> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
> index a039b99d9851d..3e47a10d72a89 100644
> --- a/drivers/iio/pressure/hid-sensor-press.c
> +++ b/drivers/iio/pressure/hid-sensor-press.c
> @@ -155,7 +155,7 @@ static const struct iio_info press_info = {
>  
>  /* Callback handler to send event after all samples are received and captured */
>  static int press_proc_event(struct hid_sensor_hub_device *hsdev,
> -				unsigned usage_id,
> +				u32 usage_id,
>  				void *priv)
>  {

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential problem
with how timestamps are handled in press_proc_event().

When a hardware timestamp isn't provided, the driver generates a software
timestamp:

	if (atomic_read(&press_state->common_attributes.data_ready)) {
		if (!press_state->timestamp)
			press_state->timestamp = iio_get_time_ns(indio_dev);

		iio_push_to_buffers_with_ts(indio_dev, &press_state->scan,
					    sizeof(press_state->scan),
					    press_state->timestamp);
	}

Because press_state->timestamp is never cleared back to 0 after pushing the
buffer, will all subsequent samples incorrectly reuse the exact same timestamp
instead of generating a new one?

[ ... ]

> @@ -176,7 +176,7 @@ static int press_proc_event(struct hid_sensor_hub_device *hsdev,
>  
>  /* Capture samples in local storage */
>  static int press_capture_sample(struct hid_sensor_hub_device *hsdev,
> -				unsigned usage_id,
> +				u32 usage_id,
>  				size_t raw_len, char *raw_data,
>  				void *priv)
>  {

[Severity: High]
This is a pre-existing issue, but could this cause unaligned accesses or
out-of-bounds reads?

Looking at press_capture_sample(), raw_data is cast directly to a pointer
without using the unaligned access helpers:

	case HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE:
		press_state->scan.press_data = *(u32 *)raw_data;
		ret = 0;
		break;
	case HID_USAGE_SENSOR_TIME_TIMESTAMP:
		press_state->timestamp = hid_sensor_convert_timestamp(
			&press_state->common_attributes, *(s64 *)raw_data);

Since raw_data points into a HID report, it might not be aligned, which could
cause undefined behavior on strictly aligned architectures.

Additionally, we're unconditionally reading 4 or 8 bytes here without checking
raw_len. If the HID descriptor specifies a smaller field size, could this read
past the end of the valid data buffer?

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