Re: [PATCH] media: venus: venc: avoid double free on video register failure

[Guangshuo Li]

On Wed, 20 May 2026 at 00:34, Bryan O'Donoghue <bod@xxxxxxxxxx> wrote:
>
> On 19/05/2026 15:58, Guangshuo Li wrote:
> > Hi Bryan,
> >
> > On Tue, 19 May 2026 at 21:20, Bryan O'Donoghue <bod@xxxxxxxxxx> wrote:
> >>
> >> Yes I take your point.
> >>
> >> So what you are describing is an error in the software contract from
> >> video_register_device() - if we look throughout the usage of that
> >> function we see either the pattern we already have - not checking for
> >> NULL or checking for NULL - not the double free case you are addressing.
> >>
> >> So really the fix - the place to litigate this is not in Venus or Iris
> >> but in video_register_device's cleanup path.
> >>
> >> ---
> >> bod
> >
> > Thanks, I agree.
> >
> > This should probably be handled in the video_register_device() failure
> > path rather than in each individual driver.
> >
> > I do not have a good idea yet for how to fix that cleanly in the v4l2
> > core. Do you have any suggestion?
>
> So if we look at how video_register_device() is used by drivers we have
> two different behaviours.
>
> 1. Trap the error and release the device
> 2. Trip the error - check for NULL and release the device
>
> Either way the _users_ of video_register_device() right now expect to
> have to call video_device_release().
>
> So... it seems to me video_register_device() also calling
> video_release() on some but not all of its error path is not the
> expected software contract.
>
> So I suggest two things.
>
> 1. Audit all users of video_register_device() and confirm the hypothesis
>     That is callers expect to own vdev and currently everybody tries
>     to clean it up.
>
> 2. If 1 is true then fix video_register_device() to not call
>     video_device_release()
>
> It either needs to be that or fully delegate ownership of vdev to
> video_device_register() _and_ update all of the callers.
>
> It may be that < 100% of callers if that is low single digits then
> worthwhile updating those drivers to match the new semantic.
>
> €0.02
>
> ---
> bod

Thanks, I agree with your suggestion.

I initially considered that some callers might not follow this ownership
semantic, so I tried to fix the issues reported by my static analysis tool
driver by driver first.

I will audit the users of video_register_device() to check the common
caller expectation, and then look into fixing the core error path if that
is the right direction.

Thanks,
Guangshuo