Re: [PATCH v2 6/8] iio: light: hid-sensor-als: Avoid race between callback setup and device exposure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free due to lack of synchronization in sensor_hub_remove_callback()
--

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

iio: light: hid-sensor-als: 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. Reorder probe and remove paths to
ensure callbacks are active before device exposure and are removed after device
is no longer accessible.

> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index ae2fda8dc500a..823bb56b9873a 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -406,25 +406,25 @@ static int hid_als_probe(struct platform_device *pdev)

[ ... ]

> -	ret = iio_device_register(indio_dev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "device register failed\n");
> -		goto error_remove_trigger;
> -	}
> -
>  	als_state->callbacks.send_event = als_proc_event;
>  	als_state->callbacks.capture_sample = als_capture_sample;
>  	als_state->callbacks.pdev = pdev;
>  	ret = sensor_hub_register_callback(hsdev, hsdev->usage, &als_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: High]
This is a pre-existing issue, but can this lead to a use-after-free if
sensor_hub_raw_event() is executing concurrently?

When iio_device_register() fails, we call sensor_hub_remove_callback() in the
new error path. Looking at sensor_hub_remove_callback() in
drivers/hid/hid-sensor-hub.c, it removes the callback list entry under
dyn_callback_lock and returns immediately without synchronizing with any
currently executing events:

drivers/hid/hid-sensor-hub.c:sensor_hub_remove_callback() {
    ...
		if (callback->usage_id == usage_id &&
						callback->hsdev == hsdev) {
			list_del(&callback->list);
			kfree(callback);
			break;
		}
	spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
    ...
}

However, sensor_hub_raw_event() drops dyn_callback_lock before executing
the callbacks:

drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
    ...
				callback->capture_sample(hsdev,
					report->field[i]->usage->hid, sz, ptr,
					callback->pdev);
    ...
}

If sensor_hub_raw_event() is running concurrently when we return from
hid_als_probe(), devres will automatically free the IIO device and als_state.
Would the concurrently executing event handler then access freed memory?

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