Re: [External] : Re: [PATCH 1/1] qemuProcessEventSubmit : fix potential use after free
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(a)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