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

[Sanjay Chitroda <sanjayembeddedse@gmail.com>]

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