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
