Re: [PATCH] media: zr364xx: fix memory leaks in probe()

Dan Carpenter
2021-01-18T12:23:33Z

On Wed, Jan 13, 2021 at 05:13:41PM +0100, Hans Verkuil wrote:
> Hi Dan,
>
> On 06/01/2021 11:10, Dan Carpenter wrote:
> > Syzbot discovered that the probe error handling doesn't clean up the
> > resources allocated in zr364xx_board_init(). There are several
> > related bugs in this code so I have re-written the error handling.
> >
> > 1) Introduce a new function zr364xx_board_uninit() which cleans up
> > the resources in zr364xx_board_init().
> > 2) In zr364xx_board_init() if the call to zr364xx_start_readpipe()
> > fails then release the "cam->buffer.frame[i].lpvbits" memory
> > before returning. This way every function either allocates
> > everything successfully or it cleans up after itself.
> > 3) Re-write the probe function so that each failure path goto frees
> > the most recent allocation. That way we don't free anything
> > before it has been allocated and we can also verify that
> > everything is freed.
> > 4) Originally, in the probe function the "cam->v4l2_dev.release"
> > pointer was set to "zr364xx_release" near the start but I moved
> > that assignment to the end, after everything had succeeded. The
> > release function was never actually called during the probe cleanup
> > process, but with this change I wanted to make it clear that we
> > don't want to call zr364xx_release() until everything is
> > allocated successfully.
> >
> > Next I re-wrote the zr364xx_release() function. Ideally this would
> > have been a simple matter of copy and pasting the cleanup code from
> > probe and adding an additional call to video_unregister_device(). But
> > there are several quirks to note.
> >
> > 1) The original code never called video_unregister_device(). In other
> > words, this is an additional bugfix.
>
> Not a bug, see below.
>

Thanks for reviewing this. I will fix a send a v2. I should have seen
that.

The layering here is sort of confusing in a way... But not anything
that needs to be dealt with immediately.

regards,
dan carpenter