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

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

Pre-existing issues:
- [High] Concurrency UAF during device unbind between `sensor_hub_raw_event` and `sensor_hub_remove_callback`.
--

--- Patch [13]: [PATCH v5 13/13] iio: pressure: hid-sensor-press: convert probe and teardown to devm-managed resources ---
commit 45fb35e2b8f4da70b5748e13b1091b8fdbd11a47
Author: Sanjay Chitroda <[email protected]>

iio: pressure: hid-sensor-press: convert probe and teardown to devm-managed resources

The commit converts HID IIO driver resource management to devm-managed helpers.
This ties the lifetime of all resources to the device and simplifies the probe
path by removing explicit error handling and teardown paths.

> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
> index 4e0f42fefd6b4..8ebb8f54a0cb9 100644
> --- a/drivers/iio/pressure/hid-sensor-press.c
> +++ b/drivers/iio/pressure/hid-sensor-press.c
[ ... ]
> @@ -288,38 +289,15 @@ static int hid_press_probe(struct platform_device *pdev)
>  	press_state->callbacks.send_event = press_proc_event;
>  	press_state->callbacks.capture_sample = press_capture_sample;
>  	press_state->callbacks.pdev = pdev;
> -	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_PRESSURE,
> -					   &press_state->callbacks);
> +	ret = devm_sensor_hub_register_callback(dev, hsdev,
> +						HID_USAGE_SENSOR_PRESSURE,
> +						&press_state->callbacks);

[Severity: High]
This is a pre-existing issue, but can unbinding this driver race with event
delivery and cause a use-after-free?

When the child driver unbinds, devm triggers sensor_hub_remove_callback(),
which removes the callback from dyn_callback_list and frees it using
dyn_callback_lock:

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

After this, devm frees the IIO device's private data (press_state).

However, there doesn't appear to be any synchronization to ensure that a
concurrently executing sensor_hub_raw_event() on another CPU has finished
using the callback pointer.

If sensor_hub_raw_event() is actively dispatching an event under pdata->lock,
could it invoke the callback with a stale pointer and write to freed memory?

drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
    ...
		callback = sensor_hub_get_callback(hdev,
				report->field[i]->physical ? report->field[i]->physical :
							     report->field[i]->application,
				report->field[i]->usage[0].collection_index,
				&hsdev, &priv);
		if (!callback) {
			ptr += sz;
			continue;
		}
...
		if (callback->capture_sample) {
			if (report->field[i]->logical)
				callback->capture_sample(hsdev,
					report->field[i]->logical, sz, ptr,
					callback->pdev);
			else
				callback->capture_sample(hsdev,
					report->field[i]->usage->hid, sz, ptr,
					callback->pdev);
		}
    ...
}

>  	if (ret < 0) {
>  		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;
>  	}
[ ... ]

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