[PATCH v3 0/3] iio: hid-sensors: fix shared callbacks in temperature and humidity

[PATCH v3 0/3] iio: hid-sensors: fix shared callbacks in temperature and humidity

[Christopher Hoover]

hid-sensor-temperature and hid-sensor-humidity share one static
struct hid_sensor_hub_callbacks across instances and overwrite its pdev
on every probe. With two temperature sensors, reports for one go to
the other's pdev; once that device is removed,
temperature_capture_sample() dereferences NULL. I hit this on 7.0
with two uhid-created sensors.

Patches 2-3 make the callbacks per instance, as the other HID sensor
drivers do. Patch 1 makes sensor_hub_remove_callback() take
pdata->lock, as sensor_hub_raw_event() does, so a report racing an
unbind cannot call through the freed callbacks.

Changes in v3:
- Patches 2-3: set the callbacks with one compound literal (Jonathan).
- Kept Joshua's Reviewed-by on all three; patches 2-3 changed only in
  style.
- Rebased onto v7.3.
- Sent as a new thread, not a reply to v2.

Changes in v2:
- New patch 1, for the use-after-free on unbind found by the Sashiko
  review of v1.

v2: https://lore.kernel.org/all/20261007184423.622445-1-ch@murgatroid.com/

Christopher Hoover (3):
  HID: hid-sensor-hub: Synchronize callback removal with raw events
  iio: temperature: hid-sensor-temperature: Use per-instance callbacks
  iio: humidity: hid-sensor-humidity: Use per-instance callbacks

 drivers/hid/hid-sensor-hub.c                     | 12 ++++++++++--
 drivers/iio/humidity/hid-sensor-humidity.c       | 14 +++++++-------
 drivers/iio/temperature/hid-sensor-temperature.c | 14 +++++++-------
 3 files changed, 24 insertions(+), 16 deletions(-)


base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.53.0

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

[Christopher Hoover]

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()
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@vger.kernel.org
Signed-off-by: Christopher Hoover <ch@murgatroid.com>
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 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);
 
 	return 0;
 }
-- 
2.53.0

[PATCH v3 2/3] iio: temperature: hid-sensor-temperature: Use per-instance callbacks

[Christopher Hoover]

hid-sensor-temperature keeps a single static struct
hid_sensor_hub_callbacks for all instances and overwrites its pdev in
every probe.  With two temperature sensors, input reports for one are
delivered with the other's platform device; once that device is
removed, platform_get_drvdata() returns NULL and
temperature_capture_sample() dereferences it:

  BUG: kernel NULL pointer dereference, address: 00000000000003a8
  RIP: 0010:temperature_capture_sample+0xd/0x50 [hid_sensor_temperature]

The oops happens in sensor_hub_raw_event() with the hub's spinlock
held, so the hub's removal then hangs.  It reproduces with two uhid
devices that each declare a temperature sensor, one destroyed while
the other still sends input reports.

Keep the callbacks in struct temperature_state, as the other HID
sensor drivers (accel, gyro, als, ...) do.

Fixes: 59d0f2da3569 ("iio: hid: Add temperature sensor support")
Cc: stable@vger.kernel.org
Signed-off-by: Christopher Hoover <ch@murgatroid.com>
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/temperature/hid-sensor-temperature.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
index 5e3262e461f4..4bc9d42a0301 100644
--- a/drivers/iio/temperature/hid-sensor-temperature.c
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -14,6 +14,7 @@
 
 struct temperature_state {
 	struct hid_sensor_common common_attributes;
+	struct hid_sensor_hub_callbacks callbacks;
 	struct hid_sensor_hub_attribute_info temperature_attr;
 	struct {
 		s32 temperature_data;
@@ -181,11 +182,6 @@ static int temperature_parse_report(struct platform_device *pdev,
 	return ret;
 }
 
-static struct hid_sensor_hub_callbacks temperature_callbacks = {
-	.send_event = &temperature_proc_event,
-	.capture_sample = &temperature_capture_sample,
-};
-
 /* Function to initialize the processing for usage id */
 static int hid_temperature_probe(struct platform_device *pdev)
 {
@@ -237,9 +233,13 @@ static int hid_temperature_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, indio_dev);
 
-	temperature_callbacks.pdev = pdev;
+	temp_st->callbacks = (struct hid_sensor_hub_callbacks) {
+		.send_event = temperature_proc_event,
+		.capture_sample = temperature_capture_sample,
+		.pdev = pdev,
+	};
 	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE,
-					   &temperature_callbacks);
+					   &temp_st->callbacks);
 	if (ret)
 		goto error_remove_trigger;
 
-- 
2.53.0

[PATCH v3 3/3] iio: humidity: hid-sensor-humidity: Use per-instance callbacks

[Christopher Hoover]

hid-sensor-humidity keeps a single static struct
hid_sensor_hub_callbacks for all instances and overwrites its pdev in
every probe, so with two humidity sensors, input reports for one are
delivered with the other's platform device, and a NULL dereference
follows once that device is removed.  The same bug was found in
hid-sensor-temperature, which shares this code.

Keep the callbacks in struct hid_humidity_state, as the other HID
sensor drivers (accel, gyro, als, ...) do.

Fixes: d7ed89d5aadf ("iio: hid: Add humidity sensor support")
Cc: stable@vger.kernel.org
Signed-off-by: Christopher Hoover <ch@murgatroid.com>
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/humidity/hid-sensor-humidity.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index 7cec81ff5685..644c82719447 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -14,6 +14,7 @@
 
 struct hid_humidity_state {
 	struct hid_sensor_common common_attributes;
+	struct hid_sensor_hub_callbacks callbacks;
 	struct hid_sensor_hub_attribute_info humidity_attr;
 	struct {
 		s32 humidity_data;
@@ -184,11 +185,6 @@ static int humidity_parse_report(struct platform_device *pdev,
 	return ret;
 }
 
-static struct hid_sensor_hub_callbacks humidity_callbacks = {
-	.send_event = &humidity_proc_event,
-	.capture_sample = &humidity_capture_sample,
-};
-
 /* Function to initialize the processing for usage id */
 static int hid_humidity_probe(struct platform_device *pdev)
 {
@@ -240,9 +236,13 @@ static int hid_humidity_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, indio_dev);
 
-	humidity_callbacks.pdev = pdev;
+	humid_st->callbacks = (struct hid_sensor_hub_callbacks) {
+		.send_event = humidity_proc_event,
+		.capture_sample = humidity_capture_sample,
+		.pdev = pdev,
+	};
 	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY,
-					   &humidity_callbacks);
+					   &humid_st->callbacks);
 	if (ret)
 		goto error_remove_trigger;
 
-- 
2.53.0

Re: [PATCH v3 2/3] iio: temperature: hid-sensor-temperature: Use per-instance callbacks

[Andy Shevchenko]

On Sun, Oct 18, 2026 at 07:34:19PM -0700, Christopher Hoover wrote:
> hid-sensor-temperature keeps a single static struct
> hid_sensor_hub_callbacks for all instances and overwrites its pdev in
> every probe.  With two temperature sensors, input reports for one are
> delivered with the other's platform device; once that device is
> removed, platform_get_drvdata() returns NULL and
> temperature_capture_sample() dereferences it:
> 
>   BUG: kernel NULL pointer dereference, address: 00000000000003a8
>   RIP: 0010:temperature_capture_sample+0xd/0x50 [hid_sensor_temperature]
> 
> The oops happens in sensor_hub_raw_event() with the hub's spinlock
> held, so the hub's removal then hangs.  It reproduces with two uhid
> devices that each declare a temperature sensor, one destroyed while
> the other still sends input reports.
> 
> Keep the callbacks in struct temperature_state, as the other HID
> sensor drivers (accel, gyro, als, ...) do.

...

>  struct temperature_state {
>  	struct hid_sensor_common common_attributes;
> +	struct hid_sensor_hub_callbacks callbacks;
>  	struct hid_sensor_hub_attribute_info temperature_attr;

To me sounds like this new member is misplaced. The _attributes and _attr
current members kinda tighten to each other, while callbacks is separate from
this "group".

>  	struct {
>  		s32 temperature_data;

No need to resend just for this, I hope Jonathan will tweak this whilst
applying.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/3] iio: humidity: hid-sensor-humidity: Use per-instance callbacks

[Andy Shevchenko]

On Sun, Oct 18, 2026 at 07:34:20PM -0700, Christopher Hoover wrote:
> hid-sensor-humidity keeps a single static struct
> hid_sensor_hub_callbacks for all instances and overwrites its pdev in
> every probe, so with two humidity sensors, input reports for one are
> delivered with the other's platform device, and a NULL dereference
> follows once that device is removed.  The same bug was found in
> hid-sensor-temperature, which shares this code.
> 
> Keep the callbacks in struct hid_humidity_state, as the other HID
> sensor drivers (accel, gyro, als, ...) do.

Same comment as per previous patch.

-- 
With Best Regards,
Andy Shevchenko

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

[Srinivas Pandruvada]

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@vger.kernel.org
> Signed-off-by: Christopher Hoover <ch@murgatroid.com>
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  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;
>  }

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

[Christopher Hoover]

>  Is this issue actually reproduced or it was found by some AI tools?

Real life.   I wedged my kernel several times with an MCP2221A usb i2c bridge.


On Wed, Oct 21, 2026 at 6:22 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> 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@vger.kernel.org
> > Signed-off-by: Christopher Hoover <ch@murgatroid.com>
> > Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > ---
> >  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;
> >  }

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

[Andy Shevchenko]

On Wed, Oct 21, 2026 at 06:21:45PM -0700, Srinivas Pandruvada wrote:
> 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".

Which is not that bad [1], right? It's consistent in the text.

> > 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.

[1]:
"Using two spaces after a period is not strictly "wrong," but it is considered
outdated and contrary to modern typographic standards."

...

Yeah, it's a slippery slope of yet another holy war, but I think this
kind of a style is a last we should care about (main point is to be
consistent in a chosen style).

-- 
With Best Regards,
Andy Shevchenko