Re: [PATCH v3 1/9] iio: hid-sensors: drop redundant iio_dev argument

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] This problem wasn't introduced by this patch, but the removal sequence across HID IIO drivers cancels asynchronous suspend requests, leaving the hardware powered on, which violates PM subsystem guidelines.
- [High] This problem wasn't introduced by this patch, but in `hid-sensor-temperature.c`, the trigger is manually unregistered and freed in the `remove` callback while the IIO device is still active and registered, creating a use-after-free vulnerability.
--

commit 774f02b73d8b538afc5af51312b8682afec55f98
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>

iio: hid-sensors: drop redundant iio_dev argument

This commit removes the unused iio_dev argument from
hid_sensor_remove_trigger() across all HID IIO drivers, adapting the
function prototype to simplify the API for a future devm-based conversion.

> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> index 2ff591b3458f7..a63dae90dadc5 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
[ ... ]
> @@ -429,7 +429,7 @@ static void hid_accel_3d_remove(struct platform_device *pdev)
>  
>  	sensor_hub_remove_callback(hsdev, hsdev->usage);
>  	iio_device_unregister(indio_dev);
> -	hid_sensor_remove_trigger(indio_dev, &accel_state->common_attributes);
> +	hid_sensor_remove_trigger(&accel_state->common_attributes);
>  }

This problem wasn't introduced by this patch, but could this removal sequence
leave the hardware powered on?

Looking at the removal path across these drivers, iio_device_unregister()
disables the IIO buffers, which eventually calls hid_sensor_power_state()
and queues an asynchronous suspend via pm_runtime_put_autosuspend().

Immediately afterward, hid_sensor_remove_trigger() calls
pm_runtime_disable(). Does calling pm_runtime_disable() immediately after an
async put cancel the pending suspend work, leaving the physical device active
while marking it suspended in the PM core?

[ ... ]
> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> index 9f628a8e5cfbe..60d4fcc8043b6 100644
> --- a/drivers/iio/temperature/hid-sensor-temperature.c
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c
[ ... ]
> @@ -265,7 +265,7 @@ static void hid_temperature_remove(struct platform_device *pdev)
>  	struct temperature_state *temp_st = iio_priv(indio_dev);
>  
>  	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
> -	hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
> +	hid_sensor_remove_trigger(&temp_st->common_attributes);
>  }

This problem also wasn't introduced by this patch, but is there a
use-after-free risk in this specific driver's removal sequence?

The temperature driver uses devm_iio_device_register() to register the IIO
device, which means the device unregistration is deferred until the devres
cleanup phase after hid_temperature_remove() returns.

However, hid_sensor_remove_trigger() is called manually here and it calls
iio_trigger_free(). Since the IIO device remains fully registered and exposed
to user-space until devres runs, could concurrent sysfs access or buffer
operations dereference the freed trigger pointer?

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