[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