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@xxxxxxxxxx>
> > ---
> > 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
