[libvirt] [PATCH 3/3] parallels: fix possible crash in case of errors in prlsdkLoadDomain

Dmitry Guryanov dguryanov at virtuozzo.com
Thu May 21 07:04:14 UTC 2015



On 05/18/2015 05:44 PM, Maxim Nestratov wrote:
> Cleanup code in prlsdkLoadDomain doesn't take into account the fact
> if private domain freeing function is assigned or not. In case it is,
> we shouldn't call it manually because virDomainObjListRemove calls it.
> Also, allocated def structure should be freed only if it's not
> assigned to domain. Otherwise it will be called twice one time by
> virDomainObjListRemove and the second by prlsdkLoadDomain itself.

Libvirt now can assign this pointer inside virDomainObjListAdd. I think 
you just need to set virDomainXMLOptionPtr->privateData->free to our 
prlsdkDomObjFreePrivate.

You can give virDomainXMLPrivateDataCallbacks structure as second 
argument of virDomainXMLOptionNew, and it will create appropriate 
virDomainXMLOption structure.

>
> Signed-off-by: Maxim Nestratov <mnestratov at parallels.com>
> ---
>   src/parallels/parallels_sdk.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
> index 5c15e94..2f2574e 100644
> --- a/src/parallels/parallels_sdk.c
> +++ b/src/parallels/parallels_sdk.c
> @@ -1379,10 +1379,21 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
>   
>       return dom;
>    error:
> -    if (dom && !olddom)
> +    if (dom && !olddom) {
> +        /* Domain isn't persistent means that we haven't yet set
> +         * prlsdkDomObjFreePrivate and should call it manually
> +         */
> +        if (!dom->persistent)
> +            prlsdkDomObjFreePrivate(pdom);
> +
>           virDomainObjListRemove(privconn->domains, dom);
> -    virDomainDefFree(def);
> -    prlsdkDomObjFreePrivate(pdom);
> +    }
> +    /* Delete newly allocated def only if we haven't assigned it to domain
> +     * Otherwise we will end up with domain having invalid def within it
> +     */
> +    if (!dom)
> +        virDomainDefFree(def);
> +
>       return NULL;
>   }
>   




More information about the libvir-list mailing list