Re: [PATCH 09/10] Input: synaptics-rmi4 - change report_size to size_t in F54

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The v4l2 device structure (vdev, queue, and lock) is embedded within devm-allocated f54_data, causing a use-after-free and locking of a freed mutex on device close if userspace holds the device open past removal.
--

commit f2033554c93a59d85d1cecf56a498ef9493e4022
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: synaptics-rmi4 - change report_size to size_t in F54

Change report_size from int to size_t in struct f54_data and update all
its usages. This avoids signed/unsigned comparison warnings when
comparing with size_t variables and matches the signature of
rmi_read_block() which takes size_t for length.

> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index 850e1742c480a..5d045ece3c2f2 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
> @@ -105,7 +105,7 @@ struct f54_data {
>  	enum rmi_f54_report_type report_type;
>  	u8 *report_data;
>  	size_t max_report_size;
> -	int report_size;
> +	size_t report_size;
>  	int report_error;
>  
>  	bool is_busy;

[Severity: High]
This is a pre-existing issue, but could embedding the V4L2 device structures
inside the devm-allocated f54_data lead to a use-after-free?

If a user holds the /dev/videoX node open during driver unbind,
rmi_f54_remove() completes and the devm framework frees the f54_data memory:

drivers/input/rmi4/rmi_f54.c:rmi_f54_remove() {
    struct f54_data *f54 = dev_get_drvdata(&fn->dev);

    video_unregister_device(&f54->vdev);
    v4l2_device_unregister(&f54->v4l2);
    cancel_delayed_work_sync(&f54->work);
    destroy_workqueue(f54->workqueue);
}

When the user later closes the file descriptor, it triggers the V4L2
release path. v4l2_release() calling vb2_fop_release() accesses the
embedded queue and attempts to acquire the vdev->queue->lock, which
points to the freed f54->lock:

drivers/input/rmi4/rmi_f54.c:rmi_f54_probe() {
    ...
    f54 = devm_kzalloc(&fn->dev, sizeof(struct f54_data), GFP_KERNEL);
    ...
    f54->vdev = rmi_f54_video_device;
    f54->vdev.v4l2_dev = &f54->v4l2;
    f54->vdev.lock = &f54->lock;
    ...
}

Should the video_device be dynamically allocated and reference counted rather
than relying on devm management?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626051802.4033172-1-dmitry.torokhov@gmail.com?part=9