Re: [libvirt] [PATCH 3/3] parallels: fix possible crash in case of errors in prlsdkLoadDomain
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(a)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;
}