Re: [PATCH v5 05/13] iio: accel: hid-sensor-accel-3d: 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:
- [Critical] Concurrent HID sensor events can trigger a Use-After-Free because callback deregistration does not synchronize with active event handlers.
--

--- Patch [5]: [PATCH v5 05/13] iio: accel: hid-sensor-accel-3d: convert probe and teardown to devm-managed resources ---
commit a9b078923d03e5b1bd623e5152abd36454ac1fc5
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>

iio: accel: hid-sensor-accel-3d: convert probe and teardown to devm-managed resources
    
Convert HID IIO driver resource management to use devm-managed helpers,
tying the lifetime of all resources to the device.

> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> index a7cccbaa82f86..7ba0228f18b43 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c

[ ... ]

> @@ -384,38 +385,14 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
>  	accel_state->callbacks.send_event = accel_3d_proc_event;
>  	accel_state->callbacks.capture_sample = accel_3d_capture_sample;
>  	accel_state->callbacks.pdev = pdev;
> -	ret = sensor_hub_register_callback(hsdev, hsdev->usage,
> -					   &accel_state->callbacks);
> +	ret = devm_sensor_hub_register_callback(dev, hsdev, hsdev->usage,
> +						&accel_state->callbacks);

[Severity: Critical]
This is a pre-existing issue, but does this sequence leave a use-after-free
window during device teardown?

If the driver unbinds or probe fails, devres will call
sensor_hub_remove_callback(). This removes the callback from the list using
dyn_callback_lock, but it does not wait for active sensor_hub_raw_event()
invocations to complete.

If sensor_hub_raw_event() is running concurrently and has already obtained the
callback pointer, devres will proceed to free the indio_dev memory.

When the event loop subsequently invokes the callback, wouldn't
accel_3d_capture_sample() execute on the freed memory?

drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
    ...
    if (callback->capture_sample) {
        if (report->field[i]->logical)
            callback->capture_sample(...);
    ...
}

Could this lead to out-of-bounds writes when it attempts to store the
sample data into the freed accel_state?

drivers/iio/accel/hid-sensor-accel-3d.c:accel_3d_capture_sample() {
    ...
    struct iio_dev *indio_dev = platform_get_drvdata(priv);
    struct accel_3d_state *accel_state = iio_priv(indio_dev);
    ...
    accel_state->scan.accel_val[CHANNEL_SCAN_INDEX_X + offset] =
                        *(u32 *)raw_data;
    ...
}

>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "callback reg failed\n");
> -		goto error_remove_trigger;
> -	}

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