Re: [PATCH v5 03/13] HID: hid-sensor-hub: introduce device managed API

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

Pre-existing issues:
- [High] Lack of synchronization in `sensor_hub_remove_callback` allows `sensor_hub_raw_event` to access a freed `usage_callback` during driver unbind.
--

--- Patch [3]: [PATCH v5 03/13] HID: hid-sensor-hub: introduce device managed API ---
commit 1f72201981d4eabf709dd34d77a678b2f0acfcf2
Author: Sanjay Chitroda <[email protected]>

HID: hid-sensor-hub: introduce device managed API

This commit introduces devm_sensor_hub_register_callback() to handle
resource acquisition and release using the device managed framework. It
adds a devres action that automatically unregisters the sensor hub
callback when the device is detached or unbound.

> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
[ ... ]
> @@ -187,6 +187,38 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
>  }
>  EXPORT_SYMBOL_GPL(sensor_hub_remove_callback);
>  
> +static void sensor_hub_remove_callback_helper(void *ptr)
> +{
> +	struct sensor_hub_cb_devres *res = ptr;
> +
> +	sensor_hub_remove_callback(res->hsdev, res->usage_id);
> +}

[Severity: High]
This isn't a bug introduced by this patch, but does
sensor_hub_remove_callback() lack synchronization with ongoing raw events?

When a client driver is unbound via sysfs, this helper will call
sensor_hub_remove_callback() to clean up. While the remove function
acquires pdata->dyn_callback_lock to unlink the callback, it returns
immediately without acquiring pdata->lock.

This means it does not wait for ongoing events to finish:

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

Meanwhile, sensor_hub_raw_event() can be actively executing on another CPU,
having already dropped dyn_callback_lock:

drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
        ...
        callback->capture_sample(hsdev, ...);
        ...
}

Because the unbind process continues immediately after the removal returns,
it can free the memory containing the usage_callback (such as through
devres_release_all()).

Could this lead to a use-after-free in sensor_hub_raw_event() when it
dereferences the freed callback pointer?

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