Re: [PATCH v2 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] Unsafe casting and blind dereferencing of `raw_data` causes unaligned memory accesses and Endianness data corruption.
- [High] Failure to reset the timestamp variable causes stale timestamps to be pushed continuously.
--

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

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

This patch changes the usage_id parameter type from unsigned to u32 in
callback functions. This aligns the signature with the expected callback
API and improves code clarity without making functional changes.

> 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)
>  {
>  	struct iio_dev *indio_dev = platform_get_drvdata(priv);

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing issue
with how the timestamp is handled in press_proc_event().

If the timestamp is zero, it gets populated with the current system time:

press_proc_event() {
        ...
        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);
        }
        ...
}

However, the driver doesn't seem to reset press_state->timestamp back to zero
after pushing the buffer. Will this cause the first recorded software timestamp
to be continuously reused for all subsequent readings if the hardware does not
provide its own timestamp?

[ ... ]

> @@ -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 also a pre-existing issue, but while looking at press_capture_sample(),
could the direct cast of raw_data cause memory alignment or endianness problems?

press_capture_sample() {
        ...
        switch (usage_id) {
        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);
                break;
        ...
}

Since raw_data points directly into the packed HID report buffer, which is
unaligned, does directly casting it to a u32 or s64 pointer violate C
alignment rules and risk kernel panics on architectures without unaligned
memory access support?

Additionally, since HID reports are little endian, wouldn't casting directly
on a big endian platform reverse the byte order? For example, if reading a
16-bit field on a big endian system, could this place the actual data into
the upper 16 bits of the u32, resulting in garbage data being sent to
userspace?

[ ... ]

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