Re: [PATCH 07/11] iio: temperature: hid-sensor-temperature: use common device for devres

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free of IIO channels array when in-kernel consumers hold references past driver unbind.
- [High] State corruption due to overwriting a static global callbacks structure during probe.
--

commit 92f73ed0d62d832f9bf6d6b90aa841b134a7d14d
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>

iio: temperature: hid-sensor-temperature: use common device for devres

This commit updates hid-sensor-temperature to allocate memory using the
platform device instead of the IIO device. This unifies devres ownership
under the platform device for deterministic resource release.

> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> index 9f628a8e5cfbe..31ebfd1a1fe2c 100644
> --- a/drivers/iio/temperature/hid-sensor-temperature.c
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c
> @@ -213,8 +213,8 @@ static int hid_temperature_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	temp_chans = devm_kmemdup(&indio_dev->dev, temperature_channels,
> -				sizeof(temperature_channels), GFP_KERNEL);
> +	temp_chans = devm_kmemdup(&pdev->dev, temperature_channels,
> +				  sizeof(temperature_channels), GFP_KERNEL);

[Severity: High]
This isn't a bug introduced by this patch, but does using devm_kmemdup()
for the IIO channels array tie its lifecycle to driver unbind rather than
the IIO device's reference-counted lifecycle?

If an in-kernel consumer holds a reference to the IIO device after the
provider driver unbinds, the consumer might access the freed channels array
through its cached iio_channel pointer (e.g., via
iio_read_channel_ext_info(), which does not check if indio_dev->info was
cleared).

Could this lead to a use-after-free? Would it be safer to make the
channels array part of the iio_priv structure so it lives as long as the
indio_dev itself?

>  	if (!temp_chans)
>  		return -ENOMEM;

[Severity: High]
This is a pre-existing issue, but looking further down in
hid_temperature_probe(), is it safe to assign pdev to a global static
callbacks structure?

hid_temperature_probe() {
    ...
    temperature_callbacks.pdev = pdev;
    ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE,
                    &temperature_callbacks);
    ...
}

If a system contains multiple HID temperature sensors, would probing a
second sensor overwrite the pdev pointer with the new device instance?

When the sensor hub receives an event for the first sensor, it might fire
the callback using the pdev of the second sensor, pushing data into the
wrong device's buffer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com?part=7