Re: [PATCH v2 1/7] HID: sensor: hub: Synchronize callback removal with raw events

[Jonathan Cameron <jic23@kernel.org>]

On Wed,  4 Nov 2026 12:13:57 -0800
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> Raw-event processing holds pdata->lock across callback lookup and
> invocation, and takes dyn_callback_lock during lookup. Removal must
> prevent both active callbacks and future lookups before client state can
> be released.
> 
> Take pdata->lock before dyn_callback_lock, matching raw-event order,
> and unlink the callback while holding both locks. This ensures active
> raw-event callbacks have finished and prevents future lookups.
> 
> Fixes: 401ca24fb34a ("HID: sensors: introduce sensor framework")
> Reported-by: Семён Марьясин <simeon@maryasin.name>
> Closes: https://lore.kernel.org/all/CAM2dg7ewfVJSP4rFyhkCB1Rmd1+X56sAg=eUdbPthG2xrLqwQQ@mail.gmail.com/
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Maybe a minimal fix is good for now, but I do note that we can
make this code nicer with some guard() magic.

Perhaps a follow up series to tidy this up.

Things are getting very messy around this driver wrt to fix vs
cleanup.  Hopefully it won't cause too much of a problem in next
until I can merge the fixes back after they go upstream.

> ---
>  drivers/hid/hid-sensor-hub.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 6470a290ebfc..0c4884c5821f 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -173,7 +173,9 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
>  	struct sensor_hub_data *pdata = hid_get_drvdata(hsdev->hdev);
>  	unsigned long flags;
>  
> -	spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
> +	/* Serialize callback removal with raw-event callback execution. */
> +	spin_lock_irqsave(&pdata->lock, flags);
> +	spin_lock(&pdata->dyn_callback_lock);

Do both of those with appropriate guard() 

>  	list_for_each_entry(callback, &pdata->dyn_callback_list, list)
>  		if (callback->usage_id == usage_id &&
>  						callback->hsdev == hsdev) {
I'd just go long!
 		if (callback->usage_id == usage_id && callback->hsdev == hsdev) {


> @@ -181,7 +183,8 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
>  			kfree(callback);
>  			break;
And we can return here.

>  		}
> -	spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
> +	spin_unlock(&pdata->dyn_callback_lock);
> +	spin_unlock_irqrestore(&pdata->lock, flags);
>  
>  	return 0;
>  }