Re: [PATCH 9/9] iio: accel: hid-sensor-accel-3d: Fix race between callback registration 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] The patch introduces a Use-After-Free / NULL Pointer Dereference in the `probe` error path by registering the sensor hub callback before IIO device registration.

Pre-existing issues:
- [High] Reordering IIO device unregistration before callback removal exacerbates a Use-After-Free vulnerability during driver teardown.
- [High] Reordering IIO device unregistration before callback removal widens a severe data race leading to linked list corruption.
--

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

iio: accel: hid-sensor-accel-3d: Fix race between callback registration and device exposure

The driver registers the IIO device before completing sensor hub
callback registration and unregisters callbacks while the IIO device
is still exposed during teardown.

> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> index 2bf05ab5235ef..c000e001c0015 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
[ ... ]
> @@ -399,13 +393,19 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
>  					&accel_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;

[Severity: High]
Does this new error path expose asynchronous event handlers to a
use-after-free or NULL pointer dereference?

When sensor_hub_register_callback() succeeds, it allows the hardware to
immediately dispatch events to accel_3d_proc_event(). If iio_device_register()
then fails, the code jumps to error_remove_callback.

While sensor_hub_remove_callback() removes the callback from the list, it
does not appear to synchronize with or wait for concurrently running handlers.
When probe() returns, devres frees the indio_dev structure and the driver
core clears drvdata.

Any concurrently executing accel_3d_proc_event() might then fetch the cleared
drvdata (causing a NULL pointer dereference) or access the freed indio_dev
(causing a use-after-free).

>  	}
>  
>  	return ret;
>  
> -error_iio_unreg:
> -	iio_device_unregister(indio_dev);
> +error_remove_callback:
> +	sensor_hub_remove_callback(hsdev, hsdev->usage);
>  error_remove_trigger:
>  	hid_sensor_remove_trigger(indio_dev, &accel_state->common_attributes);
>  	return ret;
> @@ -418,8 +418,8 @@ static void hid_accel_3d_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct accel_3d_state *accel_state = iio_priv(indio_dev);
>  
> -	sensor_hub_remove_callback(hsdev, hsdev->usage);
>  	iio_device_unregister(indio_dev);

[Severity: High]
This is a pre-existing issue, but does moving iio_device_unregister() before
sensor_hub_remove_callback() widen a data race leading to linked list
corruption?

By calling iio_device_unregister() first, the sensor callback remains fully
active and eligible to receive new events during IIO teardown.

Hardware events trigger accel_3d_proc_event(), which calls
iio_push_to_buffers(). This traverses the buffer_list without holding locks
using list_for_each_entry(). Concurrently, iio_device_unregister() calls
iio_disable_all_buffers(), which modifies the same buffer_list.

Can this concurrent list modification corrupt the traversal pointer,
potentially causing an infinite loop or a panic?

> +	sensor_hub_remove_callback(hsdev, hsdev->usage);

[Severity: High]
This is a pre-existing issue, but does this reordering exacerbate a
use-after-free vulnerability during driver teardown?

sensor_hub_remove_callback() unlinks the callback from the internal list
without synchronizing with currently executing event handlers. When the
remove function returns, devres will immediately free indio_dev.

Previously, the slow iio_device_unregister() call was executed after callback
removal, providing an implicit grace period for in-flight events to finish.
By moving it before callback removal, this delay is eliminated, which might
cause any concurrent event handler to dereference the freed indio_dev
structure more reliably.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=9