Re: [PATCH v3 1/3] HID: hid-sensor-hub: Synchronize callback removal with raw events

From: Srinivas Pandruvada

Date: Wed Oct 21 2026 - 21:22:09 EST


On Sun, 2026-10-18 at 19:34 -0700, Christopher Hoover wrote:
> sensor_hub_raw_event() holds pdata->lock while it looks a callback up
> in dyn_callback_list and calls its capture_sample() and send_event().
> sensor_hub_remove_callback() takes only dyn_callback_lock, so it can
> unlink a callback and return while another CPU is between the lookup
> and the call.  The sensor drivers remove their callback in .remove()

Two spaces before "The".

> and keep the struct hid_sensor_hub_callbacks in memory that devres
> frees right after, so a report racing an unbind can call through
> freed function pointers.
>
> Take pdata->lock in sensor_hub_remove_callback() too, outside
> dyn_callback_lock as in sensor_hub_raw_event(), so the removal waits
> for a report in flight.  pdata->lock is otherwise only taken in
> sensor_hub_raw_event(), and callbacks are removed from process
> context.
>
> Fixes: 401ca24fb34a ("HID: sensors: introduce sensor framework")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Christopher Hoover <ch@xxxxxxxxxxxxxx>
> Reviewed-by: Joshua Crofts <joshua.crofts1@xxxxxxxxx>
> ---
>  drivers/hid/hid-sensor-hub.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> hub.c
> index 6470a290ebfc..7cf9f398592e 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -173,7 +173,14 @@ 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);
> + /*
> + * sensor_hub_raw_event() holds pdata->lock while it looks
> up a
> + * callback and calls it.  Taking it here too means no
> report is
> + * still using the callback once it is unlinked, so its
> owner may
> + * free it as soon as this returns.
> + */
> + spin_lock_irqsave(&pdata->lock, flags);
> + spin_lock(&pdata->dyn_callback_lock);
>   list_for_each_entry(callback, &pdata->dyn_callback_list,
> list)
>   if (callback->usage_id == usage_id &&
>   callback->hsdev ==
> hsdev) {
> @@ -181,7 +188,8 @@ int sensor_hub_remove_callback(struct
> hid_sensor_hub_device *hsdev,
>   kfree(callback);
>   break;
>   }
> - spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
> + spin_unlock(&pdata->dyn_callback_lock);
> + spin_unlock_irqrestore(&pdata->lock, flags);
>  

The patch looks fine. Is this issue actually reproduced or it was found
by some AI tools?

Thanks,
Srinivas

>   return 0;
>  }