[PATCH v1] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()

[PATCH v1] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Avoid using devm_iio_device_register(), as this driver requires explicit
error handling and teardown ordering.

Mixing devm_* APIs with goto-based error unwinding breaks the expected
LIFO resource release model and can introduce race windows during device
removal. In particular, the IIO device may remain visible to userspace
while dependent resources are already being freed, potentially leading
to use-after-free issues.

Add explicit iio_device_unregister() call in the teardown path to ensure
deterministic cleanup and follow kernel resource management conventions.

Fixes: 59d0f2da3569 ("iio: hid: Add temperature sensor support")
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
 drivers/iio/temperature/hid-sensor-temperature.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
index 9f628a8e5cfb..34bff7e9f3a3 100644
--- a/drivers/iio/temperature/hid-sensor-temperature.c
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -244,7 +244,7 @@ static int hid_temperature_probe(struct platform_device *pdev)
 	if (ret)
 		goto error_remove_trigger;
 
-	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
+	ret = iio_device_register(indio_dev);
 	if (ret)
 		goto error_remove_callback;
 
@@ -264,6 +264,7 @@ static void hid_temperature_remove(struct platform_device *pdev)
 	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
 	struct temperature_state *temp_st = iio_priv(indio_dev);
 
+	iio_device_unregister(indio_dev);
 	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
 	hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
 }
-- 
--

Re: [PATCH v1] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Concurrent execution of HID sensor hub callbacks and device unbind leads to a Use-After-Free of `indio_dev`.
- [High] The static global `temperature_callbacks` struct is mutated during probe, causing data corruption and cross-device routing bugs when multiple sensors are present.
--

commit 829c19b8a370c5d8b09345d4e07b865deb47a88f
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>

iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()

This commit replaces devm_iio_device_register() with iio_device_register() 
to ensure deterministic cleanup. It adds an explicit iio_device_unregister() 
call in the remove path to fix potential use-after-free race conditions 
during device removal.

> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> index 9f628a8e5cfbe..34bff7e9f3a33 100644
> --- a/drivers/iio/temperature/hid-sensor-temperature.c
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c
> @@ -244,7 +244,7 @@ static int hid_temperature_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto error_remove_trigger;
>  
> -	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> +	ret = iio_device_register(indio_dev);

[Severity: High]
This isn't a bug introduced by this patch, but looking at the probe function
just prior to this registration, is there a risk of corrupting device routing
when multiple sensors are present?

The driver defines temperature_callbacks as a static global struct:

static struct hid_sensor_hub_callbacks temperature_callbacks = {
        .send_event = &temperature_proc_event,
        .capture_sample = &temperature_capture_sample,
};

During hid_temperature_probe(), the current device's pdev is directly
assigned to temperature_callbacks.pdev.

If a system has multiple HID temperature sensors, does the last probed sensor
overwrite this global pointer? It seems like when sensor_hub_raw_event()
processes hardware events, it might fetch the shared struct and pass the
single overwritten pdev to temperature_capture_sample(). Would this cause the
data from all temperature sensors to be written exclusively into the IIO state
of the last probed sensor?

[ ... ]

> @@ -264,6 +264,7 @@ static void hid_temperature_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct temperature_state *temp_st = iio_priv(indio_dev);
>  
> +	iio_device_unregister(indio_dev);
>  	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);

[Severity: Critical]
This is a pre-existing issue, but I wanted to ask about the teardown order
here.

In hid_temperature_remove(), we call sensor_hub_remove_callback(), which
removes the callback from the sensor hub's list under dyn_callback_lock.
However, it doesn't appear to synchronize with or wait for concurrently
executing callback handlers like sensor_hub_raw_event() to finish before
returning.

Since indio_dev is allocated with devm_iio_device_alloc() during probe, it
will be freed by devres immediately after hid_temperature_remove() returns.

If sensor_hub_raw_event() is executing concurrently and has already retrieved
the callback pointer, could it pass the pdev to temperature_capture_sample()
and perform a use-after-free write on the freed private state?

For example:
temp_st->scan.temperature_data = *(s32 *)raw_data;

Should we be flushing or waiting for running callbacks to complete before
letting the device context be freed?

>  	hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622052135.1804135-1-sanjayembedded@gmail.com?part=1

Re: [PATCH v1] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()

[Andy Shevchenko]

On Mon, Jun 22, 2026 at 10:51:35AM +0530, Sanjay Chitroda wrote:

> Avoid using devm_iio_device_register(), as this driver requires explicit
> error handling and teardown ordering.
> 
> Mixing devm_* APIs with goto-based error unwinding breaks the expected
> LIFO resource release model and can introduce race windows during device
> removal. In particular, the IIO device may remain visible to userspace
> while dependent resources are already being freed, potentially leading
> to use-after-free issues.
> 
> Add explicit iio_device_unregister() call in the teardown path to ensure
> deterministic cleanup and follow kernel resource management conventions.

Strictly speaking this needs Cc: stable@ as well as mentioned in the
documentation.

LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko