[libvirt PATCH] qemu: fix release of virDomainObjPtr in SSH key APIs

Michal Privoznik mprivozn at redhat.com
Mon Jan 25 07:51:25 UTC 2021


On 1/22/21 6:08 PM, Daniel P. Berrangé wrote:
> The qemuDomainObjFromDomain() API must be paired with
> the virDomainObjEndAPI API. The qemuDomainAuthorizedSSHKeysGet
> method simply did 'return -1' leaking a reference in two paths.
> 
> The qemuDomainAuthorizedSSHKeysSet method marked the object
> as an autoptr while also have some code paths that will call
> virDomainObjEndAPI, resulting in attempted double free.

I think it's not a reference that's leaking, but a lock. 
qemuDomainObjFromDomain() returns a dom object that was locked and 
ref'ed. The g_autoptr() handles the unrefing but not unlocking.

Also, because virDomainObjEndAPI() clears out the pointer, the autounref 
callback does nothing, so there shouldn't be double free.

> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com> > ---
>   src/qemu/qemu_driver.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 027617deef..05e021cce4 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20254,10 +20254,10 @@ qemuDomainAuthorizedSSHKeysGet(virDomainPtr dom,
>           return -1;
>   
>       if (virDomainAuthorizedSshKeysGetEnsureACL(dom->conn, vm->def) < 0)
> -        return -1;
> +        goto cleanup;
>   
>       if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
> -        return -1;
> +        goto cleanup;
>   

IOW, if either of these error conditions is hit then the domain is left 
locked. The patch is correct, but the error message is misleading.

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal




More information about the libvir-list mailing list