Re: [PATCH 02/11] iio: humidity: hid-sensor-humidity: align parenthesis for readability

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Global state corruption due to shared static `humidity_callbacks` struct
- [High] Out-of-bounds read and unaligned memory access in `humidity_capture_sample()`
- [High] Possible list corruption and use-after-free during device removal
--

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

iio: humidity: hid-sensor-humidity: align parenthesis for readability

Adjust alignment of parentheses to improve readability and
maintain consistency with kernel coding style

> diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
> index be2338d5f4070..6fed019f4099f 100644
> --- a/drivers/iio/humidity/hid-sensor-humidity.c
> +++ b/drivers/iio/humidity/hid-sensor-humidity.c
[ ... ]
> @@ -141,8 +141,8 @@ static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
>  
>  /* Capture samples in local storage */
>  static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
> -				unsigned int usage_id, size_t raw_len,
> -				char *raw_data, void *pdev)
> +				   unsigned int usage_id, size_t raw_len,
> +				   char *raw_data, void *pdev)

[Severity: High]
This is a pre-existing issue, but is there a potential out-of-bounds read
and unaligned memory access here?

The HID sensor hub core invokes humidity_capture_sample() with raw_data
pointing to a field within a raw byte report buffer, and raw_len specifying
its exact byte size:

drivers/iio/humidity/hid-sensor-humidity.c:humidity_capture_sample() {
    ...
	case HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY:
		humid_st->scan.humidity_data = *(s32 *)raw_data;

		return 0;
    ...
}

The driver unconditionally casts raw_data to a 32-bit integer pointer and
performs a 4-byte read. If raw_len is less than 4 (e.g., 1 or 2 bytes) and
the field resides at the end of the report buffer, does this result in an
out-of-bounds memory read?

Additionally, since HID reports are byte-packed, raw_data is not guaranteed
to be 32-bit aligned. Could an unaligned 32-bit read trigger a kernel panic
on architectures that strictly enforce alignment (e.g., ARMv5, SPARC)?

>  {
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct hid_humidity_state *humid_st = iio_priv(indio_dev);
[ ... ]
> @@ -234,7 +234,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
>  	atomic_set(&humid_st->common_attributes.data_ready, 0);
>  
>  	ret = hid_sensor_setup_trigger(indio_dev, name,
> -				&humid_st->common_attributes);
> +				       &humid_st->common_attributes);
>  	if (ret)
>  		return ret;
>  
> @@ -242,7 +242,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
>  
>  	humidity_callbacks.pdev = pdev;
>  	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY,
> -					&humidity_callbacks);

[Severity: High]
This isn't a bug introduced by this patch, but could this cause global state
corruption?

The humidity_callbacks struct is defined statically at the file scope. Here,
during hid_humidity_probe(), its pdev field is overwritten with the
instance-specific pdev.

The HID core (sensor_hub_register_callback()) stores the pointer to this
shared struct rather than copying its contents. If multiple humidity sensors
are probed, the global pdev will be overwritten by the last probed instance.

When sensor_hub_raw_event() processes an event, does it access callback->pdev
and erroneously route events from all connected sensors to the IIO device of
the last probed sensor, potentially causing data corruption or a
Use-After-Free if that device is later unbound?

> +					   &humidity_callbacks);
>  	if (ret)
>  		goto error_remove_trigger;
>  

[Severity: High]
This is a pre-existing issue, but is there a possible list corruption and
use-after-free during device removal?

In hid_humidity_remove(), iio_device_unregister() is called before
sensor_hub_remove_callback(). This allows the HID sensor hub to continue
dispatching hardware events via the active callback while the IIO device is
being concurrently torn down.

When an event arrives, humidity_proc_event() checks the data_ready atomic
variable. Since this is an unlocked Time-Of-Check to Time-Of-Use race,
could data_ready be read as non-zero right before iio_device_unregister()
disables the trigger and clears it?

humidity_proc_event() then calls iio_push_to_buffers_with_timestamp(),
which iterates over the IIO buffer_list without taking locks.

Concurrently, iio_device_unregister() calls iio_disable_all_buffers()
which calls list_del_init() on the buffers. Does traversing a list
concurrently while elements are being removed and reinitialized cause
infinite loops or Use-After-Free crashes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com?part=2