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

[sashiko-bot@kernel.org]

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

New issues:
- [High] Changing the devm allocation parent from the IIO device to the platform device creates a dangling pointer, as the IIO device can outlive the platform device.

Pre-existing issues:
- [High] The driver shares a single static global `struct hid_sensor_hub_callbacks humidity_callbacks` which is overwritten by each probed device, causing data corruption and Use-After-Free.
--

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

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

This patch changes the devres allocation parent for the humidity channels
array. It moves the ownership from the IIO device to the HID platform
device to consolidate the devres ownership tree.

> diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
> index 6fed019f4099f..73120b87d641d 100644
> --- a/drivers/iio/humidity/hid-sensor-humidity.c
> +++ b/drivers/iio/humidity/hid-sensor-humidity.c
> @@ -215,7 +215,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	humid_chans = devm_kmemdup(&indio_dev->dev, humidity_channels,
> +	humid_chans = devm_kmemdup(&pdev->dev, humidity_channels,
>  				   sizeof(humidity_channels), GFP_KERNEL);

[Severity: High]
Does changing the devm allocation parent to the platform device create a
dangling pointer here?

In the IIO subsystem, the iio_dev object is reference-counted and can outlive
the platform device if userspace holds open file descriptors (for example,
/dev/iio:deviceX). Devres resources attached to pdev->dev are freed
unconditionally when the driver unbinds, but resources attached to
indio_dev->dev live until the iio_dev reference count drops to zero.

By moving the allocation of humidity_channels to pdev->dev, won't
indio_dev->channels point to freed memory immediately after unbind, leading
to a use-after-free if the IIO core or userspace continues to interact with
the still-alive iio_dev?

>  	if (!humid_chans)
>  		return -ENOMEM;

[ ... ]

While looking at hid_humidity_probe(), I noticed another detail:

drivers/iio/humidity/hid-sensor-humidity.c:hid_humidity_probe() {
	...
	humidity_callbacks.pdev = pdev;
	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY,
					   &humidity_callbacks);
	...
}

[Severity: High]
This is a pre-existing issue, but does assigning the local platform device to
the globally shared humidity_callbacks structure cause state corruption?

If a system has multiple humidity sensors probed, it appears the pdev of the
last probed sensor overwrites the pointer for all previous instances.

When sensor_hub_raw_event() dispatches an event for an earlier sensor, it
calls callback->capture_sample() with the overwritten pdev. Could this
corrupt the state of the newer sensor, or cause a use-after-free if the
newer sensor is unbound while the older one remains active?

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