Re: [PATCH v2 5/8] iio: light: hid-sensor-prox: 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:
- [Critical] The reordering introduces a Use-After-Free race condition in the probe error path due to a lack of synchronization when removing the callback.
- [Critical] Reordering `sensor_hub_remove_callback()` after `iio_device_unregister()` creates a severe race condition where lockless list traversal races with IIO buffer teardown, potentially causing an infinite loop.

Pre-existing issues:
- [Critical] Preexisting Use-After-Free in the device remove path due to lack of callback synchronization.
--

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

iio: light: hid-sensor-prox: Avoid race between callback setup and device exposure

The driver currently exposes the IIO device to userspace before completing
sensor hub callback registration, and similarly removes callbacks while the
device can still be accessed during teardown. This reorders probe and remove
paths to avoid a race window.

> diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> index 9059f00f0cedf..11609dc4c5dc6 100644
> --- a/drivers/iio/light/hid-sensor-prox.c
> +++ b/drivers/iio/light/hid-sensor-prox.c

[ ... ]

> @@ -325,13 +319,19 @@ static int hid_prox_probe(struct platform_device *pdev)
>  					   &prox_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, hsdev->usage);

[Severity: Critical]
Could this introduce a use-after-free if the callback fires during error
handling?

Looking at sensor_hub_remove_callback(), it unlinks the callback from the
list but does not appear to wait for actively executing handlers to finish.
If a sensor event triggers concurrently, hid_prox_probe() could return,
allowing the devres framework to free the indio_dev and prox_state memory.

Would an actively executing callback then dereference this freed memory?

>  error_remove_trigger:
>  	hid_sensor_remove_trigger(indio_dev, &prox_state->common_attributes);
>  	return ret;
> @@ -344,8 +344,8 @@ static void hid_prox_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct prox_state *prox_state = iio_priv(indio_dev);
>  
> -	sensor_hub_remove_callback(hsdev, hsdev->usage);
>  	iio_device_unregister(indio_dev);
> +	sensor_hub_remove_callback(hsdev, hsdev->usage);

[Severity: Critical]
Does reordering these calls create a race condition with IIO buffer teardown?

By unregistering the IIO device before removing the sensor hub callback, a
concurrently firing callback could invoke iio_push_to_buffers() while
iio_device_unregister() is tearing down the buffers.

iio_push_to_buffers() traverses the buffer list locklessly:

    list_for_each_entry(buf, &iio_dev_opaque->buffer_list, buffer_list) {

At the same time, iio_device_unregister() modifies this list using
list_del_init() via iio_buffer_deactivate(). If the lockless iterator
evaluates a buffer node exactly when list_del_init() resets its pointers to
point to itself, could this cause an endless loop and a hard CPU stall?

[Severity: Critical]
This is a pre-existing issue, but does sensor_hub_remove_callback() properly
synchronize with active callbacks before returning?

Since sensor_hub_remove_callback() unlinks the callback but does not wait
for executions of sensor_hub_raw_event() to finish, hid_prox_remove() can
return while callbacks are still running.

Once the function returns, the devres framework frees indio_dev and
prox_state. Can this lead to a use-after-free when the still-running
callback attempts to access the freed memory?

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

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