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

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

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@linux.intel.com/
> 
>>
>> 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