[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()

[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

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

[Maxwell Doose]

On Mon, Jun 22, 2026 at 12:21 AM Sanjay Chitroda
<sanjayembeddedse@gmail.com> wrote:
>
> 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(-)
>

My rb will be a tad redundant but

Reviewed-by: Maxwell Doose <m32285159@gmail.com>

and should Cc stable as Andy said but hopefully Jonathan can amend :)

-- 
best regards,
max

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

[srinivas pandruvada]

On Mon, 2026-06-22 at 10:51 +0530, Sanjay Chitroda wrote:
> 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.

Please explain this use after free case here.

Thanks,
Srinivas

> 
> 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()

[Maxwell Doose]

On Mon, Jun 22, 2026 at 10:26 AM srinivas pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Mon, 2026-06-22 at 10:51 +0530, Sanjay Chitroda wrote:
> > 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.
>
> Please explain this use after free case here.
>
> Thanks,
> Srinivas

My guess is that because the device would still be registered but
would actually be removed, sysfs still has "wild" pointers to
read_raw() and write_raw() (which don't exist anymore), causing the
UAF. If I'm wrong feel free to correct me though.

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

[srinivas pandruvada]

On Mon, 2026-06-22 at 10:27 -0500, Maxwell Doose wrote:
> On Mon, Jun 22, 2026 at 10:26 AM srinivas pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > 
> > On Mon, 2026-06-22 at 10:51 +0530, Sanjay Chitroda wrote:
> > > 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.
> > 
> > Please explain this use after free case here.
> > 
> > Thanks,
> > Srinivas
> 
> My guess is that because the device would still be registered but
> would actually be removed, sysfs still has "wild" pointers to
> read_raw() and write_raw() (which don't exist anymore), causing the
> UAF. If I'm wrong feel free to correct me though.

iio_device_unregister() will be last one to be called after device
removal from devm action handler. This will cleanup attributes. So,
read_raw() or write_raw() can be called. The problem can be handlers
for read_raw() and write_raw() if anything there which are dependent on
clean done by hid_temperature_remove(). Here callbacks are cleaned up,
so nothing to respond to read  sensor_hub_input_attr_get_raw_value(),
so it has to wait for 5 seconds to timeout, which is not great. So
nothing against change done here.

But still not sure any use after free case, unless I am missing
something.

Thanks,
Srinivas

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

[Jonathan Cameron]

On Mon, 22 Jun 2026 13:50:22 -0700
srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> On Mon, 2026-06-22 at 10:27 -0500, Maxwell Doose wrote:
> > On Mon, Jun 22, 2026 at 10:26 AM srinivas pandruvada
> > <srinivas.pandruvada@linux.intel.com> wrote:  
> > > 
> > > On Mon, 2026-06-22 at 10:51 +0530, Sanjay Chitroda wrote:  
> > > > 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.  
> > > 
> > > Please explain this use after free case here.
> > > 
> > > Thanks,
> > > Srinivas  
> > 
> > My guess is that because the device would still be registered but
> > would actually be removed, sysfs still has "wild" pointers to
> > read_raw() and write_raw() (which don't exist anymore), causing the
> > UAF. If I'm wrong feel free to correct me though.  
> 
> iio_device_unregister() will be last one to be called after device
> removal from devm action handler. This will cleanup attributes. So,
> read_raw() or write_raw() can be called. The problem can be handlers
> for read_raw() and write_raw() if anything there which are dependent on
> clean done by hid_temperature_remove(). Here callbacks are cleaned up,
> so nothing to respond to read  sensor_hub_input_attr_get_raw_value(),
> so it has to wait for 5 seconds to timeout, which is not great. So
> nothing against change done here.
> 
> But still not sure any use after free case, unless I am missing
> something.
> 
Agreed that to call UAF you need an explained path (and preferably
testing that it happens).  The timeout issue Srinivas calls out is
sufficient for us to merge this as a fix, but the patch description
should then talk about that.

Thanks,

Jonathan
> Thanks,
> Srinivas
> 
> 

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

[Sanjay Chitroda]

On 24 June 2026 12:36:16 am IST, Jonathan Cameron <jic23@kernel.org> wrote:
>On Mon, 22 Jun 2026 13:50:22 -0700
>srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>
>> On Mon, 2026-06-22 at 10:27 -0500, Maxwell Doose wrote:
>> > On Mon, Jun 22, 2026 at 10:26 AM srinivas pandruvada
>> > <srinivas.pandruvada@linux.intel.com> wrote:  
>> > > 
>> > > On Mon, 2026-06-22 at 10:51 +0530, Sanjay Chitroda wrote:  
>> > > > 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.  
>> > > 
>> > > Please explain this use after free case here.
>> > > 
>> > > Thanks,
>> > > Srinivas  
>> > 
>> > My guess is that because the device would still be registered but
>> > would actually be removed, sysfs still has "wild" pointers to
>> > read_raw() and write_raw() (which don't exist anymore), causing the
>> > UAF. If I'm wrong feel free to correct me though.  
>> 
>> iio_device_unregister() will be last one to be called after device
>> removal from devm action handler. This will cleanup attributes. So,
>> read_raw() or write_raw() can be called. The problem can be handlers
>> for read_raw() and write_raw() if anything there which are dependent on
>> clean done by hid_temperature_remove(). Here callbacks are cleaned up,
>> so nothing to respond to read  sensor_hub_input_attr_get_raw_value(),
>> so it has to wait for 5 seconds to timeout, which is not great. So
>> nothing against change done here.
>> 
>> But still not sure any use after free case, unless I am missing
>> something.
>> 
>Agreed that to call UAF you need an explained path (and preferably
>testing that it happens).  The timeout issue Srinivas calls out is
>sufficient for us to merge this as a fix, but the patch description
>should then talk about that.
>
Thank you all for the review.

I investigated the remove path in more detail. I agree that my original use-after-free explanation was not sufficiently justified.

The issue is actually the teardown ordering. With "devm_iio_device_register()", the IIO device remains registered until the devres cleanup phase, while "remove()" first removes the sensor hub callback. During this window, the IIO device is still visible to userspace and "read_raw()" requests may be issued. These requests eventually wait for a response from the sensor hub callback, but the callback has already been removed, resulting in the operation timing out.

I'll update the commit message in v2 to describe this observable behaviour instead of mentioning a potential use-after-free. The code change itself remains the same.

Thanks, Sanjay

>Thanks,
>
>Jonathan
>> Thanks,
>> Srinivas
>> 
>> 
>