Re: [PATCH 02/10] Input: synaptics-rmi4 - zero report size on F54 work error

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] V4L2 structures are embedded in a `devm` allocated struct, leading to a Use-After-Free if a file descriptor is kept open after device unbind.
--

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

Input: synaptics-rmi4 - zero report size on F54 work error

This commit fixes a heap buffer overflow in the F54 diagnostic code. It
modifies the error path in rmi_f54_work() to ensure the report_size field is
always set to 0 on failure, preventing stale larger payload sizes from being
copied into shrunken v4l2 buffers.

> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index 61909e1a39e24..8eac320c43e35 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
[ ... ]
> @@ -588,16 +588,16 @@ static void rmi_f54_work(struct work_struct *work)
>  		if (error) {
>  			dev_err(&fn->dev, "%s: read [%d bytes] returned %d\n",
>  				__func__, size, error);
> -			goto abort;
> +			goto out;
>  		}
>  	}
>  
> -abort:
> -	f54->report_size = error ? 0 : report_size;
> -error:
> +out:
>  	if (error)
>  		report_size = 0;
>  
> +	f54->report_size = report_size;
> +

[Severity: High]
This is a pre-existing issue, but since we are modifying the v4l2-related f54
struct, is it safe to embed the video_device in a devm allocated struct?

Looking at rmi_f54_probe() and rmi_f54_remove():

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;
    ...
    ret = video_register_device(&f54->vdev, VFL_TYPE_TOUCH, -1);
    ...
}

drivers/input/rmi4/rmi_f54.c:rmi_f54_remove() {
    ...
    video_unregister_device(&f54->vdev);
    ...
}

When rmi_f54_remove() completes, the driver core frees the devm allocated
memory for f54. If a user keeps /dev/videoX open during unbind, the
video_device refcount remains above zero, but its memory has been freed.

Would subsequent system calls (like ioctl) from the open fd attempt to
dereference vdev->fops from the freed f54 memory, resulting in a
use-after-free?

>  	if (report_size == 0 && !error) {
>  		queue_delayed_work(f54->workqueue, &f54->work,
>  				   msecs_to_jiffies(1));

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