[External] : Re: [PATCH 1/1] qemuProcessEventSubmit : fix potential use after free

[Shaleen Bathla]

On Tue, Jan 10, 2023 at 09:00:42AM +0100, Peter Krempa wrote:
> On Tue, Jan 10, 2023 at 11:12:55 +0530, Shaleen Bathla wrote:
> > Coverity scan reports use after free issue.
> > In error case, don't free vm object as it will be unlocked+freed
> > in the parent function like qemuProcessHandleReset().
> 
> This explanation doesn't make too much sense to me ...
> 
> > 
> > Signed-off-by: Shaleen Bathla <shaleen.bathla at oracle.com>
> > ---
> >  src/qemu/qemu_process.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 9fc7eada5220..a4133b37cf22 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -287,7 +287,6 @@ qemuProcessEventSubmit(virDomainObj *vm,
> >      event->data = data;
> >  
> >      if (virThreadPoolSendJob(driver->workerPool, 0, event) < 0) {
> > -        virObjectUnref(vm);
> 
> ... this virObjectUnref() call here is to undo the virObjectRef() that
> was done couple lines above in case when we could not submit the event
> to the worker thread.
> 
> There is no code in between where we'd unlock the VM.
> 
> Could you elaborate a bit more how you see the bug happening? Ideally
> also attach the coverity report.
>
Looks like this was a false positive in coverity scan. CID in libvirt is 403592.
I use gui coverity, not sure how to attach report. Should I attach screenshot?
I think this can be eliminated if we use virObjectUnref(event->vm) instead.
I can send a v2 patch with the above change if agreed?

Regards,
Shaleen Bathla