Re: [PATCH 01/10] iio: humidity: hid-sensor-humidity: use local struct device

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver shares a single, global static `struct hid_sensor_hub_callbacks humidity_callbacks` across all device instances, leading to cross-device data corruption and potential Use-After-Free if multiple devices are probed.
--

--- Patch [1]: [PATCH 01/10] iio: humidity: hid-sensor-humidity: use local struct device ---
commit e9be49bc2265ade3aeb3c548b989cbea2029fdd4
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>

iio: humidity: hid-sensor-humidity: use local struct device

This commit introduces a local struct device pointer derived from pdev->dev
in hid_humidity_probe() to avoid repeated usage and improve readability.

> diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
> index 7cec81ff56856..1b254d3aeee90 100644
> --- a/drivers/iio/humidity/hid-sensor-humidity.c
> +++ b/drivers/iio/humidity/hid-sensor-humidity.c
> @@ -196,10 +196,11 @@ static int hid_humidity_probe(struct platform_device *pdev)

[Severity: High]
This is a pre-existing issue, but does sharing a single global static
humidity_callbacks structure across all device instances lead to cross-device
data corruption?

Looking at hid_humidity_probe(), it unconditionally overwrites the shared
static callback structure with the pdev of the most recently probed instance:

hid_humidity_probe(struct platform_device *pdev)
{
        ...
        humidity_callbacks.pdev = pdev;
        ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY,
                                           &humidity_callbacks);
        ...
}

If multiple HID humidity sensors are probed, the static humidity_callbacks
struct is overwritten. If an earlier probed device generates an event, the
callback functions will receive the wrong pdev.

Additionally, if the most recently probed device is disconnected, its pdev is
freed. If the remaining connected device generates an event, could the driver
dereference a dangling pointer?

Should the callbacks struct be allocated per-device inside the dynamically
allocated state structure instead of being defined globally as a static
variable?

>  	struct iio_dev *indio_dev;
>  	struct hid_humidity_state *humid_st;
>  	struct iio_chan_spec *humid_chans;
> -	struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
> +	struct device *dev = &pdev->dev;
> +	struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
>  	int ret;
>  
> -	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*humid_st));
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*humid_st));
>  	if (!indio_dev)
>  		return -ENOMEM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com?part=1