Re: [PATCH v2 4/8] iio: pressure: hid-sensor-press: Avoid race between callback setup and device exposure

[sashiko-bot@kernel.org]

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

New issues:
- [High] Reordering `iio_device_unregister()` before `sensor_hub_remove_callback()` introduces a race condition leading to list corruption or Use-After-Free.

Pre-existing issues:
- [High] Lack of synchronization in `sensor_hub_remove_callback()` allows concurrent events to cause a Use-After-Free in the probe error path and device removal.
- [High] Unaligned and out-of-bounds memory access in `press_capture_sample()`.
--

commit 9b0142efc7c55a69dc83b67a2c132e1f174fd7cc
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Subject: iio: pressure: hid-sensor-press: Avoid race between callback setup and device exposure

This patch reorders the probe and remove paths in the hid-sensor-press
driver. It registers the sensor hub callbacks before exposing the IIO
device to userspace, and in reverse during teardown, intending to fix
timing windows where buffers might be accessed before callbacks are ready.

> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
> index 3e47a10d72a89..8f81a6d65b9f7 100644
> --- a/drivers/iio/pressure/hid-sensor-press.c
> +++ b/drivers/iio/pressure/hid-sensor-press.c

[ ... ]

> @@ -304,13 +298,19 @@ static int hid_press_probe(struct platform_device *pdev)
>  					&press_state->callbacks);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "callback reg failed\n");
> -		goto error_iio_unreg;
> +		goto error_remove_trigger;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "device register failed\n");
> +		goto error_remove_callback;
>  	}
>  
>  	return ret;
>  
> -error_iio_unreg:
> -	iio_device_unregister(indio_dev);
> +error_remove_callback:
> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);

[Severity: High]
This is a pre-existing issue, but does sensor_hub_remove_callback() wait
for any executing callbacks to complete?

If iio_device_register() fails, the new error path calls
sensor_hub_remove_callback(). If a callback like press_proc_event() is
already running concurrently, it appears the memory for press_state will be
freed by devres while the callback is still dereferencing it, potentially
leading to a use-after-free.

>  error_remove_trigger:
>  	hid_sensor_remove_trigger(indio_dev, &press_state->common_attributes);
>  	return ret;
> @@ -323,8 +323,8 @@ static void hid_press_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct press_state *press_state = iio_priv(indio_dev);
>  
> -	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);
>  	iio_device_unregister(indio_dev);
> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);

[Severity: High]
Does unregistering the IIO device before removing the sensor hub callback
introduce a regression during teardown?

By explicitly reversing the teardown order, iio_device_unregister() will
dismantle the IIO buffers via iio_disable_all_buffers(). If a hardware
event arrives concurrently, the still-active press_proc_event() callback can
call iio_push_to_buffers_with_ts() and attempt to push data to buffers being
locklessly destroyed, which could lead to list corruption or a
use-after-free.

>  	hid_sensor_remove_trigger(indio_dev, &press_state->common_attributes);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but is press_capture_sample()
safely handling the raw_data buffer?

Looking at the callback handling in press_capture_sample():

drivers/iio/pressure/hid-sensor-press.c:press_capture_sample() {
	...
	switch (usage_id) {
	case HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE:
		press_state->scan.press_data = *(u32 *)raw_data;
        ...
}

The pointer raw_data is directly cast to u32 * and dereferenced without
verifying raw_len or checking for proper alignment.

Could a maliciously crafted HID descriptor with a shorter report length
cause an out-of-bounds read that leaks heap memory to userspace, or trigger
unaligned access faults on architectures that strictly require alignment?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-5-june-hid-iio-race-fixes-v2-0-1cfabcd1881e@gmail.com?part=4