Re: [PATCH v3 01/10] media: synopsys: hdmirx: free the driver data when the last user is gone

[Dmitry Osipenko]

On 10/16/26 14:06, Sascha Hauer wrote:
> Hi Dmitry,
>
> On 2026-10-07 14:20, Dmitry Osipenko wrote:
> > On 10/5/26 16:28, Sascha Hauer wrote:
> > > @@ -2792,16 +2807,13 @@ static void hdmirx_remove(struct platform_device *pdev)
> > >   	hdmirx_disable_irq(dev);
> > >   
> > >   	vb2_video_unregister_device(&hdmirx_dev->stream.vdev);
> > > -	v4l2_ctrl_handler_free(&hdmirx_dev->hdl);
> > >   	v4l2_device_unregister(&hdmirx_dev->v4l2_dev);
> > >   
> > > -	/* touched by hdmirx_disable()->hdmirx_plugout() */
> > > -	hdmirx_dev->rgb_range = NULL;
> > > -	hdmirx_dev->content_type = NULL;
> > > -
> > >   	hdmirx_disable(dev);
> > >   
> > >   	reset_control_bulk_assert(HDMIRX_NUM_RST, hdmirx_dev->resets);
> > > +
> > > +	v4l2_device_put(&hdmirx_dev->v4l2_dev);
> > >   }
> >
> > This is uncommon v4l driver removal pattern, I suspect it's either not
> > correct or needs to be fixed in the core code.
>
> The pattern looks uncommon because many drivers in the tree are affected
> by the same issue ;)
>
> Just like many other drivers the hdmirx embeds a struct v4l2_device in
> its private data struct which is allocated with devm_*, thus freed
> during driver removal. With an opened file descriptor the video_device
> outlives the driver removal, and v4l2_release(), called on
> the last file descriptors close then references the already freed memory
> via vdev->v4l2_dev
>
> This is a long standing problem. See [1] for example for an attempt to
> fix it in the core. It seems consencsus at that time was that it must be
> fixed in the drivers which is what this patch does for the hdmirx.
>
> [1] https://lore.kernel.org/all/20240610100530.1107771-1-sakari.ailus@xxxxxxxxxxxxxxx/
>
> > What is the user-visible or sanitizer-visible problem? How to reproduce it?
>
> It can be reproduced with Kasan on affected drivers, below on the hdmirx
> driver, but it should be reproducible like this on hantro and others as
> well.
> ...
Thanks, the patches are good to me (tested on 5b with no problems found) 
apart from this one that feels like it's not a proper solution.

Will be good to have input from the v4l core maintainers, or have this 
fix made separately from the series to not hold rest of the patches.

--
Best regards,
Dmitry