Re: [PATCH v2 5/6] iio: position: hid-sensor-custom-intel-hinge: 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] Lifetime mismatch creating a dangling pointer and latent Use-After-Free for indio_dev->channels

Pre-existing issues:
- [High] Preexisting Use-After-Free due to lack of synchronization in sensor_hub_remove_callback()
--

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

iio: position: hid-sensor-custom-intel-hinge: use common device for devres

This commit changes the devres allocation for the IIO channels array from
using the IIO device to using the platform device. The goal is to bind the
memory lifecycle to the HID platform device instead of the IIO device.

> diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
> index 1f4a40716c3f7..eb6c59f81c3f3 100644
> --- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c
> +++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
> @@ -293,7 +293,7 @@ static int hid_hinge_probe(struct platform_device *pdev)
>  	}
>  
>  	indio_dev->num_channels = ARRAY_SIZE(hinge_channels);
> -	indio_dev->channels = devm_kmemdup(&indio_dev->dev, hinge_channels,
> +	indio_dev->channels = devm_kmemdup(&pdev->dev, hinge_channels,
>  					   sizeof(hinge_channels), GFP_KERNEL);

[Severity: High]
Does binding the channels array to the platform device introduce a lifetime
issue for the IIO device?

In the IIO framework, indio_dev is reference-counted and can outlive the
platform device if an in-kernel consumer holds a reference via
iio_channel_get().

If the platform device unbinds, devres will automatically free the channels
array. However, because indio_dev remains alive, a subsequent access to
channel metadata by the consumer (such as calling
iio_get_channel_ext_info_count()) would dereference the freed channels
array.

>  	if (!indio_dev->channels)
>  		return -ENOMEM;

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing
synchronization step when removing callbacks during the error paths here
and in the driver's unbind function?

The HID sensor hub processes events in sensor_hub_raw_event(), which looks
up a callback and executes it (like hinge_capture_sample()) while holding
pdata->lock.

When sensor_hub_remove_callback() is called, it removes the callback from
the list and returns without acquiring pdata->lock or waiting for the
ongoing event processing to finish:

drivers/hid/hid-sensor-hub.c:sensor_hub_remove_callback() {
    spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
    list_for_each_entry(callback, &pdata->dyn_callback_list, list)
        if (callback->usage_id == usage_id &&
                        callback->hsdev == hsdev) {
            list_del(&callback->list);
            kfree(callback);
            break;
        }
    spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
}

If the driver is unbound concurrently via sysfs while background sensor
events are being received, devres will free the driver's private state when
the unbind completes. The concurrent sensor_hub_raw_event() could then write
raw sensor data directly into the freed state structure.

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