* Re: [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
2026-06-25 12:16 [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register() Sanjay Chitroda
@ 2026-06-25 12:39 ` sashiko-bot
2026-07-08 13:50 ` Sanjay Chitroda
2026-07-08 23:25 ` srinivas pandruvada
2 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-06-25 12:39 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver incorrectly uses a static global structure `temperature_callbacks` to hold per-device state, causing cross-device data corruption and potential Use-After-Free.
- [High] A race condition during device unbind can lead to a Use-After-Free or NULL pointer dereference because `sensor_hub_remove_callback` does not wait for concurrently executing events.
--
commit 0a06e7ba97cafc3e2c79740421bf8186cb0fc540
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
The patch switches from devm_iio_device_register() to iio_device_register()
to fix a teardown ordering issue. It adds an explicit iio_device_unregister()
in the remove function to ensure the device is not accessed after its backend
resources begin to be dismantled.
> 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 is a pre-existing issue, but does this driver incorrectly use a static
global structure for per-device state?
Looking at hid_temperature_probe(), we overwrite the shared global pdev
pointer in the statically allocated temperature_callbacks structure:
drivers/iio/temperature/hid-sensor-temperature.c:hid_temperature_probe() {
...
temperature_callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE,
&temperature_callbacks);
...
}
If multiple temperature sensors are probed, the most recently probed device
will overwrite this shared pointer. If the last probed device is later
unbound, its driver data is cleared.
Subsequent events for other active devices will dereference the cleared pdev
in temperature_proc_event(), leading to a null pointer dereference or
use-after-free.
> 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);
[Severity: High]
This isn't a bug introduced by this patch, but can a race condition during
device unbind lead to a use-after-free or null pointer dereference?
The sensor_hub_remove_callback() removes the callback from the sensor hub's
list, but does not synchronize with concurrently executing callback handlers
like sensor_hub_raw_event().
If an event from the sensor hub arrives exactly as the driver is unbinding,
the event handler might fetch the callback just before it is removed. When
hid_temperature_remove() completes, devres frees indio_dev, and the driver
core sets drvdata to NULL.
The concurrent event handler then executes temperature_capture_sample(),
calling platform_get_drvdata(pdev) which either returns NULL or a dangling
pointer to the freed indio_dev, causing a crash.
> hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625121611.2003780-1-sanjayembedded@gmail.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
2026-06-25 12:16 [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register() Sanjay Chitroda
2026-06-25 12:39 ` sashiko-bot
@ 2026-07-08 13:50 ` Sanjay Chitroda
2026-07-08 23:25 ` srinivas pandruvada
2 siblings, 0 replies; 5+ messages in thread
From: Sanjay Chitroda @ 2026-07-08 13:50 UTC (permalink / raw)
To: jikos, jic23, srinivas.pandruvada
Cc: dlechner, nuno.sa, andy, hongyan.song, linux-input, linux-iio,
linux-kernel
On 25 June 2026 5:46:11 pm IST, 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.
>
>With devm_iio_device_register(), IIO device remains registered until the
>devres cleanup phase. However, driver's remove() callback removes the
>sensor hub callback and trigger support. This can create a race window
>where IIO device is still visible and read_raw() requests are issued.
>These requests might call sensor_hub_input_attr_get_raw_value(), which
>waits up to 5 seconds for a response from the sensor hub callback that
>has already been removed.
>
>Add an explicit iio_device_unregister() call in the teardown path to
>ensure deterministic cleanup, so that userspace can no longer access the
>device once backend resources begin to be dismantled.
>
>Fixes: 59d0f2da3569 ("iio: hid: Add temperature sensor support")
>Cc: stable@vger.kernel.org
>Reviewed-by: Maxwell Doose <m32285159@gmail.com>
>Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
Hi Jonathan,
There are two pre-existing issue highlighted by Sashiko which can be analysed and fix in separate change if required.
Requesting your valuable input on this fix. also, this is pre-requisite for HID IIO devm conversation series.
Thanks, Sanjay
>---
>Changes in v2:
>- Added review tags of Maxwell and Andy along with stable Cc
>- Based on input from Srinivas and investigation use-after-free
> explanation was not sufficiently justified so updated the commit
> message with appropriate information and kept Fixes tag
>- Link to v1: https://lore.kernel.org/all/20260622052135.1804135-1-sanjayembedded@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 a8d3a15f9c53..ab6ec8f659b8 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);
> }
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
2026-06-25 12:16 [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register() Sanjay Chitroda
2026-06-25 12:39 ` sashiko-bot
2026-07-08 13:50 ` Sanjay Chitroda
@ 2026-07-08 23:25 ` srinivas pandruvada
2026-07-11 22:30 ` Jonathan Cameron
2 siblings, 1 reply; 5+ messages in thread
From: srinivas pandruvada @ 2026-07-08 23:25 UTC (permalink / raw)
To: Sanjay Chitroda, jikos, jic23
Cc: dlechner, nuno.sa, andy, hongyan.song, linux-input, linux-iio,
linux-kernel
On Thu, 2026-06-25 at 17:46 +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.
>
> With devm_iio_device_register(), IIO device remains registered until
> the
> devres cleanup phase. However, driver's remove() callback removes the
> sensor hub callback and trigger support. This can create a race
> window
> where IIO device is still visible and read_raw() requests are issued.
> These requests might call sensor_hub_input_attr_get_raw_value(),
> which
> waits up to 5 seconds for a response from the sensor hub callback
> that
> has already been removed.
>
> Add an explicit iio_device_unregister() call in the teardown path to
> ensure deterministic cleanup, so that userspace can no longer access
> the
> device once backend resources begin to be dismantled.
>
> Fixes: 59d0f2da3569 ("iio: hid: Add temperature sensor support")
> Cc: stable@vger.kernel.org
> Reviewed-by: Maxwell Doose <m32285159@gmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> Changes in v2:
> - Added review tags of Maxwell and Andy along with stable Cc
> - Based on input from Srinivas and investigation use-after-free
> explanation was not sufficiently justified so updated the commit
> message with appropriate information and kept Fixes tag
> - Link to v1:
> https://lore.kernel.org/all/20260622052135.1804135-1-sanjayembedded@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 a8d3a15f9c53..ab6ec8f659b8 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);
> }
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
2026-07-08 23:25 ` srinivas pandruvada
@ 2026-07-11 22:30 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2026-07-11 22:30 UTC (permalink / raw)
To: srinivas pandruvada
Cc: Sanjay Chitroda, jikos, dlechner, nuno.sa, andy, hongyan.song,
linux-input, linux-iio, linux-kernel
On Wed, 08 Jul 2026 16:25:41 -0700
srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> On Thu, 2026-06-25 at 17:46 +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.
> >
> > With devm_iio_device_register(), IIO device remains registered until
> > the
> > devres cleanup phase. However, driver's remove() callback removes the
> > sensor hub callback and trigger support. This can create a race
> > window
> > where IIO device is still visible and read_raw() requests are issued.
> > These requests might call sensor_hub_input_attr_get_raw_value(),
> > which
> > waits up to 5 seconds for a response from the sensor hub callback
> > that
> > has already been removed.
> >
> > Add an explicit iio_device_unregister() call in the teardown path to
> > ensure deterministic cleanup, so that userspace can no longer access
> > the
> > device once backend resources begin to be dismantled.
> >
> > Fixes: 59d0f2da3569 ("iio: hid: Add temperature sensor support")
> > Cc: stable@vger.kernel.org
> > Reviewed-by: Maxwell Doose <m32285159@gmail.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> >
>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Applied to the fixes-togreg branch of iio.git.
Note that to take anything non fix related on top of this will require
me waiting until this is upstream. I'm not sure how many fixes
I'll have outstanding (and so if I will soon reach critical mass)
but most likely that will be at least a few weeks at least.
Thanks
Jonathan
>
> > ---
> > Changes in v2:
> > - Added review tags of Maxwell and Andy along with stable Cc
> > - Based on input from Srinivas and investigation use-after-free
> > explanation was not sufficiently justified so updated the commit
> > message with appropriate information and kept Fixes tag
> > - Link to v1:
> > https://lore.kernel.org/all/20260622052135.1804135-1-sanjayembedded@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 a8d3a15f9c53..ab6ec8f659b8 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);
> > }
^ permalink raw reply [flat|nested] 5+ messages in thread