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@xxxxxxxxxxxxxxx
> > Co-developed-by: Ijae Kim <ae878000@xxxxxxxxx>
> > Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
> > Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
> > ---
> >   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@xxxxxxxxxx>

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.