Re: [PATCH v5 11/13] iio: orientation: hid-sensor-rotation: convert probe and teardown to devm-managed resources

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Race condition between sensor_hub_raw_event() and driver unbind leads to Use-After-Free.
--

--- Patch [11]: [PATCH v5 11/13] iio: orientation: hid-sensor-rotation: convert probe and teardown to devm-managed resources ---
commit 3160b0601a08ac2a6833abd2c964d88a54dc98cc
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>

iio: orientation: hid-sensor-rotation: convert probe and teardown to devm-managed resources

This patch converts the HID IIO driver resource management to use
devm-managed helpers. It transitions HID trigger setup, IIO registration,
and sensor hub callback resources to devm APIs, simplifying probe error
handling and removing the remove callback.

> diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
> index 67ffa4f4075e3..3ef89d66a88f9 100644
> --- a/drivers/iio/orientation/hid-sensor-rotation.c
> +++ b/drivers/iio/orientation/hid-sensor-rotation.c
[ ... ]
> @@ -330,38 +331,14 @@ static int hid_dev_rot_probe(struct platform_device *pdev)
>  	rot_state->callbacks.send_event = dev_rot_proc_event;
>  	rot_state->callbacks.capture_sample = dev_rot_capture_sample;
>  	rot_state->callbacks.pdev = pdev;
> -	ret = sensor_hub_register_callback(hsdev, hsdev->usage,
> -					   &rot_state->callbacks);
> +	ret = devm_sensor_hub_register_callback(dev, hsdev, hsdev->usage,
> +						&rot_state->callbacks);

[Severity: High]
This isn't a bug introduced by this patch, but does removing the callback
registration here leave us vulnerable to a use-after-free during unbind?

When a user unbinds the driver or an error occurs during probe, devres begins
cleanup and calls sensor_hub_remove_callback().

However, this function does not appear to synchronize with concurrent readers:

drivers/hid/hid-sensor-hub.c:sensor_hub_remove_callback() {
    ...
    spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
    ...
    spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
    return 0;
}

Because it doesn't take pdata->lock to wait for executing callbacks, a
concurrent HID event running in sensor_hub_raw_event() can hold a pointer to
the callback.

Devres will then proceed to free the IIO device memory (indio_dev and
rot_state).

If the concurrent event then executes callback->capture_sample(), could it
access the freed memory?

drivers/iio/orientation/hid-sensor-rotation.c:dev_rot_capture_sample() {
    struct iio_dev *indio_dev = platform_get_drvdata(priv);
    struct dev_rot_state *rot_state = iio_priv(indio_dev);
    ...
}

>  	if (ret) {
>  		dev_err(&pdev->dev, "callback reg failed\n");
> -		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;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-28-apr-iio-redundant-argument-v5-temp-v5-0-f1f92c1d830f@gmail.com?part=11