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

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

[Myeonghun Pak]

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 by neither
remove nor file release paths.

Release the queue before continuing through the existing probe error
path.

This issue was identified during our ongoing static-analysis research while
reviewing kernel code.

Fixes: 3a762dbd5347 ("[media] Input: synaptics-rmi4 - add support for F54 diagnostics")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/input/rmi4/rmi_f54.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 61909e1a39..fca7b9fec5 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);
 		goto remove_v4l2;
 	}
 
-- 
2.47.1

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

[sashiko-bot]

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

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

[Dmitry Torokhov]

On Mon, May 25, 2026 at 03:23:45AM +0900, Myeonghun Pak wrote:
> 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 by neither
> remove nor file release paths.
> 
> Release the queue before continuing through the existing probe error
> path.
> 
> This issue was identified during our ongoing static-analysis research while
> reviewing kernel code.
> 
> Fixes: 3a762dbd5347 ("[media] Input: synaptics-rmi4 - add support for F54 diagnostics")
> Cc: stable@vger.kernel.org
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
>  drivers/input/rmi4/rmi_f54.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index 61909e1a39..fca7b9fec5 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);
>  		goto remove_v4l2;
>  	}
>  

Hans, could you please Ack or Nak it? It is unclear to me if this
cleanup is mandatory and whether it is also needed in rmi_f54_remove().

Thanks.

-- 
Dmitry

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

[Hans Verkuil]

On 5/26/26 7:36 PM, Dmitry Torokhov wrote:
> On Mon, May 25, 2026 at 03:23:45AM +0900, Myeonghun Pak wrote:
>> 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 by neither
>> remove nor file release paths.
>>
>> Release the queue before continuing through the existing probe error
>> path.
>>
>> This issue was identified during our ongoing static-analysis research while
>> reviewing kernel code.
>>
>> Fixes: 3a762dbd5347 ("[media] Input: synaptics-rmi4 - add support for F54 diagnostics")
>> Cc: stable@vger.kernel.org
>> Co-developed-by: Ijae Kim <ae878000@gmail.com>
>> Signed-off-by: Ijae Kim <ae878000@gmail.com>
>> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
>> ---
>>   drivers/input/rmi4/rmi_f54.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
>> index 61909e1a39..fca7b9fec5 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);

vb2_queue_release is not needed here: since the video device was never 
created, it also never started streaming, and this call is only needed 
if streaming is in progress.

Looking at other drivers I see that in most cases they shouldn't call
vb2_queue_release at all. I need to go through the media drivers and
fix them.

In any case:

Rejected-by: Hans Verkuil <hverkuil+cisco@kernel.org>

Regards,

	Hans

>>   		goto remove_v4l2;
>>   	}
>>   
> 
> Hans, could you please Ack or Nak it? It is unclear to me if this
> cleanup is mandatory and whether it is also needed in rmi_f54_remove().
> 
> Thanks.
>