On Tue, Jan 05, 2021 at 04:37:41PM +0200, Andy Shevchenko wrote: > On Tue, Jan 05, 2021 at 05:00:45PM +0300, Dan Carpenter wrote: > > On Wed, Dec 30, 2020 at 11:19:18PM +0200, Andy Shevchenko wrote: > > > When ->probe() fails in some cases it may not free resources. > > > Replace few separated calls by v4l2_device_put() to clean up > > > everything. > > > > > > > The clean up everything style of error handling is always buggy. > > > > For example, in this case, all the early error paths will now crash > > instead of leaking. The __videobuf_free() function will Oops when it > > dereferences "q->int_ops->magic". > > > > MAGIC_CHECK(q->int_ops->magic, MAGIC_QTYPE_OPS); > > > > The "q->int_ops" pointer is set in videobuf_queue_vmalloc_init(). There > > are probably other bugs as well. It's almost impossible to audit this > > style of error handling either for completeness or for crashyness. > > Feel free to submit better fix, thanks! Sure. I'm too tired to think straight today. I see now that syzbot actually discovered the Oops in __videobuf_free() as well... I'm sort of surprised that the original code never called zr364xx_release(). We might have another reference leak somewhere... regards, dan carpenter
