Re: [PATCH] Input: rmi4 - release F54 queue on video registration failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Embedding `struct video_device` and `struct vb2_queue` inside a devm-managed structure causes a Use-After-Free if the device node is held open after unbind.
- [High] `destroy_workqueue()` is called in `rmi_f54_remove()` without first canceling the delayed work, risking a Use-After-Free.
--

commit f89d07a2e226d7667f0c3e90fb7262a70db54401
Author: Myeonghun Pak <mhun512@gmail.com>

Input: rmi4 - release F54 queue on video registration failure

rmi_f54_probe() initializes the videobuf2 queue before registering the
video device. If video_register_device() fails, probe only unregisters
the V4L2 device and leaves the initialized queue unwound.

> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index ac4041a69fcd3..d35b5164b170a 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
> @@ -722,6 +722,7 @@ static int rmi_f54_probe(struct rmi_function *fn)
>  	ret = video_register_device(&f54->vdev, VFL_TYPE_TOUCH, -1);
>  	if (ret) {
>  		dev_err(&fn->dev, "Unable to register video subdevice.");
> +		vb2_queue_release(&f54->queue);

[Severity: High]
This isn't a bug introduced by this patch, but does embedding struct
video_device and struct vb2_queue inside the devm-managed f54_data
structure cause a use-after-free if the device node is held open after
unbind?

In rmi_f54_remove(), video_unregister_device(&f54->vdev) is called, which
clears the registered flag and drops one reference to vdev. However, if a
userspace process still has the character device open, the V4L2 core has
elevated the vdev refcount.

When rmi_f54_remove() returns, f54_data is immediately freed by devres,
even though vdev still has active references. When userspace later closes
the file descriptor, v4l2_release() calls the driver's .release operation,
which dereferences the already-freed vdev and vdev->queue.

>  		goto remove_v4l2;
>  	}

[Severity: High]
This is a pre-existing issue, but is there also a use-after-free risk in
rmi_f54_remove() regarding the delayed work?

destroy_workqueue(f54->workqueue) is called in rmi_f54_remove() without
first calling cancel_delayed_work_sync(&f54->work).

destroy_workqueue() drains pending work but does not cancel or wait for
non-pending delayed_work that is only linked on the timer side. After
rmi_f54_remove() completes, f54_data is freed by devres.

When the pending timer subsequently expires, it will attempt to access the
freed f54->work structure and schedule work on the destroyed workqueue.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524182351.27658-1-mhun512@gmail.com?part=1