Re: [PATCH 3/9] iio: gyro: hid-sensor-gyro-3d: Fix race between callback registration and device exposure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lack of synchronization in sensor_hub_remove_callback() leads to Use-After-Free (UAF) of function pointers and device state.
--

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

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

This commit reorders callback registration and IIO device registration
in the probe and remove paths to prevent race conditions during
initialization and teardown.

> diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
[ ... ]
> @@ -337,13 +331,19 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
>  					&gyro_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_GYRO_3D);

[Severity: High]
Does this new error path introduce a use-after-free window if an event
arrives concurrently?

Looking at sensor_hub_remove_callback(), it removes the callback from the
active list but does not appear to synchronize with or wait for concurrently
executing event handlers.

If iio_device_register() fails, probe() calls sensor_hub_remove_callback()
and returns immediately. The devres subsystem will then free the indio_dev
and gyro_state memory.

Could this sequence occur:

CPU1
hid_gyro_3d_probe()
    error_remove_callback:
    sensor_hub_remove_callback()
    return ret;

CPU2 sees this in sensor_hub_raw_event():
    if (callback->capture_sample) {
        callback->capture_sample(...);

Since callback points to the devm-allocated gyro_state->callbacks, would
this lead to dereferencing freed function pointers and state data?

>  error_remove_trigger:
>  	hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
>  	return ret;
[ ... ]
> @@ -356,8 +356,8 @@ static void hid_gyro_3d_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
>  
> -	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
>  	iio_device_unregister(indio_dev);
> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);

[Severity: High]
Does this teardown sequence suffer from the same lack of synchronization?

If sensor_hub_remove_callback() does not wait for ongoing events to complete,
could a concurrent sensor_hub_raw_event() still be executing and access
gyro_state->callbacks after hid_gyro_3d_remove() finishes and devres cleans
up the memory?

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

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