File f44bfb7f-CVE-2011-1486.patch of Package libvirt.import4712

commit f44bfb7fb978c9313ce050a1c4149bf04aa0a670
Author: Jiri Denemark <jdenemar@redhat.com>
Date:   Wed Mar 23 16:00:58 2011 +0100

    Make error reporting in libvirtd thread safe
    
    Bug https://bugzilla.redhat.com/show_bug.cgi?id=689374 reported libvirtd
    crash during error dispatch.
    
    The reason is that libvirtd uses remoteDispatchConnError() with non-NULL
    conn parameter which means that virConnGetLastError() is used instead of
    its thread safe replacement virGetLastError().
    
    So when several libvirtd threads are reporting errors at the same time,
    the errors can get mixed or corrupted or in case of bad luck libvirtd
    itself crashes.
    
    Since Daniel B. is going to rewrite this code from scratch on top of his
    RPC infrastructure, I tried to come up with a minimal fix. Thus,
    remoteDispatchConnError() now just ignores its conn argument and always
    calls virGetLastError(). However, several callers had to be touched as
    well, since no libvirt API is allowed to be called before dispatching
    the error. Doing so would reset the error and we would have nothing to
    dispatch. As a result of that, the code is not very nice but that
    doesn't really make daemon/remote.c worse than it is now :-) And it will
    all die soon, which is good.
    
    The bug report also contains a reproducer in C which detects both mixed
    up error messages and libvirtd crash. Before this patch, I was able to
    crash libvirtd in about 20 seconds up to 3 minutes depending on number
    of CPU cores (more is better) and luck.

Index: libvirt-0.8.8/daemon/dispatch.c
===================================================================
--- libvirt-0.8.8.orig/daemon/dispatch.c
+++ libvirt-0.8.8/daemon/dispatch.c
@@ -114,14 +114,10 @@ void remoteDispatchOOMError (remote_erro
 
 
 void remoteDispatchConnError (remote_error *rerr,
-                              virConnectPtr conn)
+                              virConnectPtr conn ATTRIBUTE_UNUSED)
 {
-    virErrorPtr verr;
+    virErrorPtr verr = virGetLastError();
 
-    if (conn)
-        verr = virConnGetLastError(conn);
-    else
-        verr = virGetLastError();
     if (verr)
         remoteDispatchCopyError(rerr, verr);
     else
Index: libvirt-0.8.8/daemon/remote.c
===================================================================
--- libvirt-0.8.8.orig/daemon/remote.c
+++ libvirt-0.8.8/daemon/remote.c
@@ -757,8 +757,8 @@ remoteDispatchDomainGetSchedulerType (st
 
     type = virDomainGetSchedulerType (dom, &nparams);
     if (type == NULL) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -801,9 +801,9 @@ remoteDispatchDomainGetSchedulerParamete
 
     r = virDomainGetSchedulerParameters (dom, params, &nparams);
     if (r == -1) {
+        remoteDispatchConnError(rerr, conn);
         virDomainFree(dom);
         VIR_FREE(params);
-        remoteDispatchConnError(rerr, conn);
         return -1;
     }
 
@@ -908,12 +908,13 @@ remoteDispatchDomainSetSchedulerParamete
     }
 
     r = virDomainSetSchedulerParameters (dom, params, nparams);
-    virDomainFree(dom);
     VIR_FREE(params);
     if (r == -1) {
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
+    virDomainFree(dom);
 
     return 0;
 }
@@ -939,8 +940,8 @@ remoteDispatchDomainBlockStats (struct q
     path = args->path;
 
     if (virDomainBlockStats (dom, path, &stats, sizeof stats) == -1) {
-        virDomainFree (dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree (dom);
         return -1;
     }
     virDomainFree (dom);
@@ -975,8 +976,8 @@ remoteDispatchDomainInterfaceStats (stru
     path = args->path;
 
     if (virDomainInterfaceStats (dom, path, &stats, sizeof stats) == -1) {
-        virDomainFree (dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree (dom);
         return -1;
     }
     virDomainFree (dom);
@@ -1026,12 +1027,13 @@ remoteDispatchDomainMemoryStats (struct
     }
 
     nr_stats = virDomainMemoryStats (dom, stats, args->maxStats, 0);
-    virDomainFree (dom);
     if (nr_stats == -1) {
         VIR_FREE(stats);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree (dom);
         return -1;
     }
+    virDomainFree (dom);
 
     /* Allocate return buffer */
     if (VIR_ALLOC_N(ret->stats.stats_val, args->maxStats) < 0) {
@@ -1092,8 +1094,8 @@ remoteDispatchDomainBlockPeek (struct qe
     if (virDomainBlockPeek (dom, path, offset, size,
                             ret->buffer.buffer_val, flags) == -1) {
         /* free (ret->buffer.buffer_val); - caller frees */
-        virDomainFree (dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree (dom);
         return -1;
     }
     virDomainFree (dom);
@@ -1141,8 +1143,8 @@ remoteDispatchDomainMemoryPeek (struct q
     if (virDomainMemoryPeek (dom, offset, size,
                              ret->buffer.buffer_val, flags) == -1) {
         /* free (ret->buffer.buffer_val); - caller frees */
-        virDomainFree (dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree (dom);
         return -1;
     }
     virDomainFree (dom);
@@ -1168,8 +1170,8 @@ remoteDispatchDomainAttachDevice (struct
     }
 
     if (virDomainAttachDevice (dom, args->xml) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1194,8 +1196,8 @@ remoteDispatchDomainAttachDeviceFlags (s
     }
 
     if (virDomainAttachDeviceFlags (dom, args->xml, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1220,8 +1222,8 @@ remoteDispatchDomainUpdateDeviceFlags (s
     }
 
     if (virDomainUpdateDeviceFlags (dom, args->xml, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1246,8 +1248,8 @@ remoteDispatchDomainCreate (struct qemud
     }
 
     if (virDomainCreate (dom) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1272,8 +1274,8 @@ remoteDispatchDomainCreateWithFlags (str
     }
 
     if (virDomainCreateWithFlags (dom, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -1346,8 +1348,8 @@ remoteDispatchDomainDestroy (struct qemu
     }
 
     if (virDomainDestroy (dom) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1372,8 +1374,8 @@ remoteDispatchDomainDetachDevice (struct
     }
 
     if (virDomainDetachDevice (dom, args->xml) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -1399,8 +1401,8 @@ remoteDispatchDomainDetachDeviceFlags (s
     }
 
     if (virDomainDetachDeviceFlags (dom, args->xml, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -1428,8 +1430,8 @@ remoteDispatchDomainDumpXml (struct qemu
     /* remoteDispatchClientRequest will free this. */
     ret->xml = virDomainGetXMLDesc (dom, args->flags);
     if (!ret->xml) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1497,8 +1499,8 @@ remoteDispatchDomainGetAutostart (struct
     }
 
     if (virDomainGetAutostart (dom, &ret->autostart) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1524,8 +1526,8 @@ remoteDispatchDomainGetInfo (struct qemu
     }
 
     if (virDomainGetInfo (dom, &info) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -1559,8 +1561,8 @@ remoteDispatchDomainGetMaxMemory (struct
 
     ret->memory = virDomainGetMaxMemory (dom);
     if (ret->memory == 0) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1586,8 +1588,8 @@ remoteDispatchDomainGetMaxVcpus (struct
 
     ret->num = virDomainGetMaxVcpus (dom);
     if (ret->num == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1614,8 +1616,8 @@ remoteDispatchDomainGetSecurityLabel(str
 
     memset(&seclabel, 0, sizeof seclabel);
     if (virDomainGetSecurityLabel(dom, &seclabel) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -1686,8 +1688,8 @@ remoteDispatchDomainGetOsType (struct qe
     /* remoteDispatchClientRequest will free this */
     ret->type = virDomainGetOSType (dom);
     if (ret->type == NULL) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1737,10 +1739,10 @@ remoteDispatchDomainGetVcpus (struct qem
                                   info, args->maxinfo,
                                   cpumaps, args->maplen);
     if (info_len == -1) {
+        remoteDispatchConnError(rerr, conn);
         VIR_FREE(info);
         VIR_FREE(cpumaps);
         virDomainFree(dom);
-        remoteDispatchConnError(rerr, conn);
         return -1;
     }
 
@@ -1794,8 +1796,8 @@ remoteDispatchDomainGetVcpusFlags (struc
 
     ret->num = virDomainGetVcpusFlags (dom, args->flags);
     if (ret->num == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -1877,11 +1879,12 @@ remoteDispatchDomainMigratePerform (stru
                                  args->cookie.cookie_len,
                                  args->uri,
                                  args->flags, dname, args->resource);
-    virDomainFree (dom);
     if (r == -1) {
         remoteDispatchConnError(rerr, conn);
+        virDomainFree (dom);
         return -1;
     }
+    virDomainFree (dom);
 
     return 0;
 }
@@ -2013,8 +2016,8 @@ remoteDispatchDomainMigratePrepareTunnel
                                       args->flags, dname, args->resource,
                                       args->dom_xml);
     if (r == -1) {
-        remoteFreeClientStream(client, stream);
         remoteDispatchConnError(rerr, conn);
+        remoteFreeClientStream(client, stream);
         return -1;
     }
 
@@ -2175,8 +2178,8 @@ remoteDispatchDomainPinVcpu (struct qemu
                            (unsigned char *) args->cpumap.cpumap_val,
                            args->cpumap.cpumap_len);
     if (rv == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2201,8 +2204,8 @@ remoteDispatchDomainReboot (struct qemud
     }
 
     if (virDomainReboot (dom, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2245,8 +2248,8 @@ remoteDispatchDomainResume (struct qemud
     }
 
     if (virDomainResume (dom) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2271,8 +2274,8 @@ remoteDispatchDomainSave (struct qemud_s
     }
 
     if (virDomainSave (dom, args->to) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2297,8 +2300,8 @@ remoteDispatchDomainCoreDump (struct qem
     }
 
     if (virDomainCoreDump (dom, args->to, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2323,8 +2326,8 @@ remoteDispatchDomainSetAutostart (struct
     }
 
     if (virDomainSetAutostart (dom, args->autostart) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2349,8 +2352,8 @@ remoteDispatchDomainSetMaxMemory (struct
     }
 
     if (virDomainSetMaxMemory (dom, args->memory) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2375,8 +2378,8 @@ remoteDispatchDomainSetMemory (struct qe
     }
 
     if (virDomainSetMemory (dom, args->memory) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2465,12 +2468,13 @@ remoteDispatchDomainSetMemoryParameters(
     }
 
     r = virDomainSetMemoryParameters(dom, params, nparams, flags);
-    virDomainFree(dom);
     VIR_FREE(params);
     if (r == -1) {
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
+    virDomainFree(dom);
 
     return 0;
 }
@@ -2515,9 +2519,9 @@ remoteDispatchDomainGetMemoryParameters(
 
     r = virDomainGetMemoryParameters(dom, params, &nparams, flags);
     if (r == -1) {
+        remoteDispatchConnError(rerr, conn);
         virDomainFree(dom);
         VIR_FREE(params);
-        remoteDispatchConnError(rerr, conn);
         return -1;
     }
     /* In this case, we need to send back the number of parameters
@@ -2611,8 +2615,8 @@ remoteDispatchDomainSetVcpus (struct qem
     }
 
     if (virDomainSetVcpus (dom, args->nvcpus) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2637,8 +2641,8 @@ remoteDispatchDomainSetVcpusFlags (struc
     }
 
     if (virDomainSetVcpusFlags (dom, args->nvcpus, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2663,8 +2667,8 @@ remoteDispatchDomainShutdown (struct qem
     }
 
     if (virDomainShutdown (dom) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2689,8 +2693,8 @@ remoteDispatchDomainSuspend (struct qemu
     }
 
     if (virDomainSuspend (dom) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2715,8 +2719,8 @@ remoteDispatchDomainUndefine (struct qem
     }
 
     if (virDomainUndefine (dom) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2808,8 +2812,8 @@ remoteDispatchDomainManagedSave (struct
     }
 
     if (virDomainManagedSave (dom, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2835,8 +2839,8 @@ remoteDispatchDomainHasManagedSaveImage
 
     ret->ret = virDomainHasManagedSaveImage (dom, args->flags);
     if (ret->ret == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2861,8 +2865,8 @@ remoteDispatchDomainManagedSaveRemove (s
     }
 
     if (virDomainManagedSaveRemove (dom, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
     virDomainFree(dom);
@@ -2921,8 +2925,8 @@ remoteDispatchNetworkCreate (struct qemu
     }
 
     if (virNetworkCreate (net) == -1) {
-        virNetworkFree(net);
         remoteDispatchConnError(rerr, conn);
+        virNetworkFree(net);
         return -1;
     }
     virNetworkFree(net);
@@ -2991,8 +2995,8 @@ remoteDispatchNetworkDestroy (struct qem
     }
 
     if (virNetworkDestroy (net) == -1) {
-        virNetworkFree(net);
         remoteDispatchConnError(rerr, conn);
+        virNetworkFree(net);
         return -1;
     }
     virNetworkFree(net);
@@ -3019,8 +3023,8 @@ remoteDispatchNetworkDumpXml (struct qem
     /* remoteDispatchClientRequest will free this. */
     ret->xml = virNetworkGetXMLDesc (net, args->flags);
     if (!ret->xml) {
-        virNetworkFree(net);
         remoteDispatchConnError(rerr, conn);
+        virNetworkFree(net);
         return -1;
     }
     virNetworkFree(net);
@@ -3045,8 +3049,8 @@ remoteDispatchNetworkGetAutostart (struc
     }
 
     if (virNetworkGetAutostart (net, &ret->autostart) == -1) {
-        virNetworkFree(net);
         remoteDispatchConnError(rerr, conn);
+        virNetworkFree(net);
         return -1;
     }
     virNetworkFree(net);
@@ -3073,8 +3077,8 @@ remoteDispatchNetworkGetBridgeName (stru
     /* remoteDispatchClientRequest will free this. */
     ret->name = virNetworkGetBridgeName (net);
     if (!ret->name) {
-        virNetworkFree(net);
         remoteDispatchConnError(rerr, conn);
+        virNetworkFree(net);
         return -1;
     }
     virNetworkFree(net);
@@ -3143,8 +3147,8 @@ remoteDispatchNetworkSetAutostart (struc
     }
 
     if (virNetworkSetAutostart (net, args->autostart) == -1) {
-        virNetworkFree(net);
         remoteDispatchConnError(rerr, conn);
+        virNetworkFree(net);
         return -1;
     }
     virNetworkFree(net);
@@ -3169,8 +3173,8 @@ remoteDispatchNetworkUndefine (struct qe
     }
 
     if (virNetworkUndefine (net) == -1) {
-        virNetworkFree(net);
         remoteDispatchConnError(rerr, conn);
+        virNetworkFree(net);
         return -1;
     }
     virNetworkFree(net);
@@ -3406,8 +3410,8 @@ remoteDispatchInterfaceGetXmlDesc (struc
     /* remoteDispatchClientRequest will free this. */
     ret->xml = virInterfaceGetXMLDesc (iface, args->flags);
     if (!ret->xml) {
-        virInterfaceFree(iface);
         remoteDispatchConnError(rerr, conn);
+        virInterfaceFree(iface);
         return -1;
     }
     virInterfaceFree(iface);
@@ -3454,8 +3458,8 @@ remoteDispatchInterfaceUndefine (struct
     }
 
     if (virInterfaceUndefine (iface) == -1) {
-        virInterfaceFree(iface);
         remoteDispatchConnError(rerr, conn);
+        virInterfaceFree(iface);
         return -1;
     }
     virInterfaceFree(iface);
@@ -3480,8 +3484,8 @@ remoteDispatchInterfaceCreate (struct qe
     }
 
     if (virInterfaceCreate (iface, args->flags) == -1) {
-        virInterfaceFree(iface);
         remoteDispatchConnError(rerr, conn);
+        virInterfaceFree(iface);
         return -1;
     }
     virInterfaceFree(iface);
@@ -3506,8 +3510,8 @@ remoteDispatchInterfaceDestroy (struct q
     }
 
     if (virInterfaceDestroy (iface, args->flags) == -1) {
-        virInterfaceFree(iface);
         remoteDispatchConnError(rerr, conn);
+        virInterfaceFree(iface);
         return -1;
     }
     virInterfaceFree(iface);
@@ -4420,8 +4424,8 @@ remoteDispatchStoragePoolCreate (struct
     }
 
     if (virStoragePoolCreate (pool, args->flags) == -1) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
     virStoragePoolFree(pool);
@@ -4490,8 +4494,8 @@ remoteDispatchStoragePoolBuild (struct q
     }
 
     if (virStoragePoolBuild (pool, args->flags) == -1) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
     virStoragePoolFree(pool);
@@ -4517,8 +4521,8 @@ remoteDispatchStoragePoolDestroy (struct
     }
 
     if (virStoragePoolDestroy (pool) == -1) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
     virStoragePoolFree(pool);
@@ -4543,8 +4547,8 @@ remoteDispatchStoragePoolDelete (struct
     }
 
     if (virStoragePoolDelete (pool, args->flags) == -1) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
     virStoragePoolFree(pool);
@@ -4569,8 +4573,8 @@ remoteDispatchStoragePoolRefresh (struct
     }
 
     if (virStoragePoolRefresh (pool, args->flags) == -1) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
     virStoragePoolFree(pool);
@@ -4596,8 +4600,8 @@ remoteDispatchStoragePoolGetInfo (struct
     }
 
     if (virStoragePoolGetInfo (pool, &info) == -1) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
 
@@ -4631,8 +4635,8 @@ remoteDispatchStoragePoolDumpXml (struct
     /* remoteDispatchClientRequest will free this. */
     ret->xml = virStoragePoolGetXMLDesc (pool, args->flags);
     if (!ret->xml) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
     virStoragePoolFree(pool);
@@ -4657,8 +4661,8 @@ remoteDispatchStoragePoolGetAutostart (s
     }
 
     if (virStoragePoolGetAutostart (pool, &ret->autostart) == -1) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
     virStoragePoolFree(pool);
@@ -4729,11 +4733,12 @@ remoteDispatchStoragePoolLookupByVolume
     }
 
     pool = virStoragePoolLookupByVolume (vol);
-    virStorageVolFree(vol);
     if (pool == NULL) {
         remoteDispatchConnError(rerr, conn);
+        virStorageVolFree(vol);
         return -1;
     }
+    virStorageVolFree(vol);
 
     make_nonnull_storage_pool (&ret->pool, pool);
     virStoragePoolFree(pool);
@@ -4758,8 +4763,8 @@ remoteDispatchStoragePoolSetAutostart (s
     }
 
     if (virStoragePoolSetAutostart (pool, args->autostart) == -1) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
     virStoragePoolFree(pool);
@@ -4784,8 +4789,8 @@ remoteDispatchStoragePoolUndefine (struc
     }
 
     if (virStoragePoolUndefine (pool) == -1) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
     virStoragePoolFree(pool);
@@ -4927,11 +4932,12 @@ remoteDispatchStorageVolCreateXml (struc
     }
 
     vol = virStorageVolCreateXML (pool, args->xml, args->flags);
-    virStoragePoolFree(pool);
     if (vol == NULL) {
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
+    virStoragePoolFree(pool);
 
     make_nonnull_storage_vol (&ret->vol, vol);
     virStorageVolFree(vol);
@@ -4958,19 +4964,21 @@ remoteDispatchStorageVolCreateXmlFrom (s
 
     clonevol = get_nonnull_storage_vol (conn, args->clonevol);
     if (clonevol == NULL) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
 
     newvol = virStorageVolCreateXMLFrom (pool, args->xml, clonevol,
                                          args->flags);
-    virStorageVolFree(clonevol);
-    virStoragePoolFree(pool);
     if (newvol == NULL) {
         remoteDispatchConnError(rerr, conn);
+        virStorageVolFree(clonevol);
+        virStoragePoolFree(pool);
         return -1;
     }
+    virStorageVolFree(clonevol);
+    virStoragePoolFree(pool);
 
     make_nonnull_storage_vol (&ret->vol, newvol);
     virStorageVolFree(newvol);
@@ -4995,8 +5003,8 @@ remoteDispatchStorageVolDelete (struct q
     }
 
     if (virStorageVolDelete (vol, args->flags) == -1) {
-        virStorageVolFree(vol);
         remoteDispatchConnError(rerr, conn);
+        virStorageVolFree(vol);
         return -1;
     }
     virStorageVolFree(vol);
@@ -5054,8 +5062,8 @@ remoteDispatchStorageVolGetInfo (struct
     }
 
     if (virStorageVolGetInfo (vol, &info) == -1) {
-        virStorageVolFree(vol);
         remoteDispatchConnError(rerr, conn);
+        virStorageVolFree(vol);
         return -1;
     }
 
@@ -5088,8 +5096,8 @@ remoteDispatchStorageVolDumpXml (struct
     /* remoteDispatchClientRequest will free this. */
     ret->xml = virStorageVolGetXMLDesc (vol, args->flags);
     if (!ret->xml) {
-        virStorageVolFree(vol);
         remoteDispatchConnError(rerr, conn);
+        virStorageVolFree(vol);
         return -1;
     }
     virStorageVolFree(vol);
@@ -5117,8 +5125,8 @@ remoteDispatchStorageVolGetPath (struct
     /* remoteDispatchClientRequest will free this. */
     ret->name = virStorageVolGetPath (vol);
     if (!ret->name) {
-        virStorageVolFree(vol);
         remoteDispatchConnError(rerr, conn);
+        virStorageVolFree(vol);
         return -1;
     }
     virStorageVolFree(vol);
@@ -5145,11 +5153,12 @@ remoteDispatchStorageVolLookupByName (st
     }
 
     vol = virStorageVolLookupByName (pool, args->name);
-    virStoragePoolFree(pool);
     if (vol == NULL) {
         remoteDispatchConnError(rerr, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
+    virStoragePoolFree(pool);
 
     make_nonnull_storage_vol (&ret->vol, vol);
     virStorageVolFree(vol);
@@ -5386,8 +5395,8 @@ remoteDispatchNodeDeviceNumOfCaps (struc
 
     ret->num = virNodeDeviceNumOfCaps(dev);
     if (ret->num < 0) {
-        virNodeDeviceFree(dev);
         remoteDispatchConnError(rerr, conn);
+        virNodeDeviceFree(dev);
         return -1;
     }
 
@@ -5432,8 +5441,8 @@ remoteDispatchNodeDeviceListCaps (struct
         virNodeDeviceListCaps (dev, ret->names.names_val,
                                args->maxnames);
     if (ret->names.names_len == -1) {
-        virNodeDeviceFree(dev);
         remoteDispatchConnError(rerr, conn);
+        virNodeDeviceFree(dev);
         VIR_FREE(ret->names.names_val);
         return -1;
     }
@@ -5462,8 +5471,8 @@ remoteDispatchNodeDeviceDettach (struct
     }
 
     if (virNodeDeviceDettach(dev) == -1) {
-        virNodeDeviceFree(dev);
         remoteDispatchConnError(rerr, conn);
+        virNodeDeviceFree(dev);
         return -1;
     }
 
@@ -5491,8 +5500,8 @@ remoteDispatchNodeDeviceReAttach (struct
     }
 
     if (virNodeDeviceReAttach(dev) == -1) {
-        virNodeDeviceFree(dev);
         remoteDispatchConnError(rerr, conn);
+        virNodeDeviceFree(dev);
         return -1;
     }
 
@@ -5520,8 +5529,8 @@ remoteDispatchNodeDeviceReset (struct qe
     }
 
     if (virNodeDeviceReset(dev) == -1) {
-        virNodeDeviceFree(dev);
         remoteDispatchConnError(rerr, conn);
+        virNodeDeviceFree(dev);
         return -1;
     }
 
@@ -5572,8 +5581,8 @@ remoteDispatchNodeDeviceDestroy(struct q
     }
 
     if (virNodeDeviceDestroy(dev) == -1) {
-        virNodeDeviceFree(dev);
         remoteDispatchConnError(rerr, conn);
+        virNodeDeviceFree(dev);
         return -1;
     }
 
@@ -5953,8 +5962,8 @@ static int remoteDispatchDomainIsActive(
     ret->active = virDomainIsActive(domain);
 
     if (ret->active < 0) {
-        virDomainFree(domain);
         remoteDispatchConnError(err, conn);
+        virDomainFree(domain);
         return -1;
     }
 
@@ -5981,8 +5990,8 @@ static int remoteDispatchDomainIsPersist
     ret->persistent = virDomainIsPersistent(domain);
 
     if (ret->persistent < 0) {
-        virDomainFree(domain);
         remoteDispatchConnError(err, conn);
+        virDomainFree(domain);
         return -1;
     }
 
@@ -6009,8 +6018,8 @@ static int remoteDispatchDomainIsUpdated
     ret->updated = virDomainIsUpdated(domain);
 
     if (ret->updated < 0) {
-        virDomainFree(domain);
         remoteDispatchConnError(err, conn);
+        virDomainFree(domain);
         return -1;
     }
 
@@ -6037,8 +6046,8 @@ static int remoteDispatchInterfaceIsActi
     ret->active = virInterfaceIsActive(iface);
 
     if (ret->active < 0) {
-        virInterfaceFree(iface);
         remoteDispatchConnError(err, conn);
+        virInterfaceFree(iface);
         return -1;
     }
 
@@ -6065,8 +6074,8 @@ static int remoteDispatchNetworkIsActive
     ret->active = virNetworkIsActive(network);
 
     if (ret->active < 0) {
-        virNetworkFree(network);
         remoteDispatchConnError(err, conn);
+        virNetworkFree(network);
         return -1;
     }
 
@@ -6093,8 +6102,8 @@ static int remoteDispatchNetworkIsPersis
     ret->persistent = virNetworkIsPersistent(network);
 
     if (ret->persistent < 0) {
-        virNetworkFree(network);
         remoteDispatchConnError(err, conn);
+        virNetworkFree(network);
         return -1;
     }
 
@@ -6121,8 +6130,8 @@ static int remoteDispatchStoragePoolIsAc
     ret->active = virStoragePoolIsActive(pool);
 
     if (ret->active < 0) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(err, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
 
@@ -6149,8 +6158,8 @@ static int remoteDispatchStoragePoolIsPe
     ret->persistent = virStoragePoolIsPersistent(pool);
 
     if (ret->persistent < 0) {
-        virStoragePoolFree(pool);
         remoteDispatchConnError(err, conn);
+        virStoragePoolFree(pool);
         return -1;
     }
 
@@ -6245,8 +6254,8 @@ remoteDispatchDomainGetJobInfo (struct q
     }
 
     if (virDomainGetJobInfo (dom, &info) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -6287,8 +6296,8 @@ remoteDispatchDomainAbortJob (struct qem
     }
 
     if (virDomainAbortJob (dom) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -6316,8 +6325,8 @@ remoteDispatchDomainMigrateSetMaxDowntim
     }
 
     if (virDomainMigrateSetMaxDowntime(dom, args->downtime, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -6346,8 +6355,8 @@ remoteDispatchDomainSnapshotCreateXml (s
 
     snapshot = virDomainSnapshotCreateXML(domain, args->xml_desc, args->flags);
     if (snapshot == NULL) {
-        virDomainFree(domain);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(domain);
         return -1;
     }
 
@@ -6388,12 +6397,12 @@ remoteDispatchDomainSnapshotDumpXml (str
     rc = 0;
 
 cleanup:
+    if (rc < 0)
+        remoteDispatchConnError(rerr, conn);
     if (snapshot)
         virDomainSnapshotFree(snapshot);
     if (domain)
         virDomainFree(domain);
-    if (rc < 0)
-        remoteDispatchConnError(rerr, conn);
 
     return rc;
 }
@@ -6417,8 +6426,8 @@ remoteDispatchDomainSnapshotNum (struct
 
     ret->num = virDomainSnapshotNum(domain, args->flags);
     if (ret->num == -1) {
-        virDomainFree(domain);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(domain);
         return -1;
     }
 
@@ -6462,9 +6471,9 @@ remoteDispatchDomainSnapshotListNames (s
                                                       args->nameslen,
                                                       args->flags);
     if (ret->names.names_len == -1) {
+        remoteDispatchConnError(rerr, conn);
         virDomainFree(domain);
         VIR_FREE(ret->names.names_val);
-        remoteDispatchConnError(rerr, conn);
         return -1;
     }
 
@@ -6493,8 +6502,8 @@ remoteDispatchDomainSnapshotLookupByName
 
     snapshot = virDomainSnapshotLookupByName(domain, args->name, args->flags);
     if (snapshot == NULL) {
-        virDomainFree(domain);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(domain);
         return -1;
     }
 
@@ -6526,8 +6535,8 @@ remoteDispatchDomainHasCurrentSnapshot(s
 
     result = virDomainHasCurrentSnapshot(domain, args->flags);
     if (result < 0) {
-        virDomainFree(domain);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(domain);
         return -1;
     }
 
@@ -6558,8 +6567,8 @@ remoteDispatchDomainSnapshotCurrent(stru
 
     snapshot = virDomainSnapshotCurrent(domain, args->flags);
     if (snapshot == NULL) {
-        virDomainFree(domain);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(domain);
         return -1;
     }
 
@@ -6598,12 +6607,12 @@ remoteDispatchDomainRevertToSnapshot (st
     rc = 0;
 
 cleanup:
+    if (rc < 0)
+        remoteDispatchConnError(rerr, conn);
     if (snapshot)
         virDomainSnapshotFree(snapshot);
     if (domain)
         virDomainFree(domain);
-    if (rc < 0)
-        remoteDispatchConnError(rerr, conn);
 
     return rc;
 }
@@ -6635,12 +6644,12 @@ remoteDispatchDomainSnapshotDelete (stru
     rc = 0;
 
 cleanup:
+    if (rc < 0)
+        remoteDispatchConnError(rerr, conn);
     if (snapshot)
         virDomainSnapshotFree(snapshot);
     if (domain)
         virDomainFree(domain);
-    if (rc < 0)
-        remoteDispatchConnError(rerr, conn);
 
     return rc;
 }
@@ -6805,8 +6814,8 @@ remoteDispatchNwfilterUndefine (struct q
     }
 
     if (virNWFilterUndefine (nwfilter) == -1) {
-        virNWFilterFree(nwfilter);
         remoteDispatchConnError(rerr, conn);
+        virNWFilterFree(nwfilter);
         return -1;
     }
     virNWFilterFree(nwfilter);
@@ -6868,8 +6877,8 @@ remoteDispatchNwfilterGetXmlDesc (struct
     /* remoteDispatchClientRequest will free this. */
     ret->xml = virNWFilterGetXMLDesc (nwfilter, args->flags);
     if (!ret->xml) {
-        virNWFilterFree(nwfilter);
         remoteDispatchConnError(rerr, conn);
+        virNWFilterFree(nwfilter);
         return -1;
     }
     virNWFilterFree(nwfilter);
@@ -6916,8 +6925,8 @@ remoteDispatchDomainGetBlockInfo (struct
     }
 
     if (virDomainGetBlockInfo (dom, args->path, &info, args->flags) == -1) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         return -1;
     }
 
@@ -6949,8 +6958,8 @@ qemuDispatchMonitorCommand (struct qemud
 
     if (virDomainQemuMonitorCommand(domain, args->cmd, &ret->result,
                                     args->flags) == -1) {
-        virDomainFree(domain);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(domain);
         return -1;
     }
 
@@ -6993,15 +7002,15 @@ remoteDispatchDomainOpenConsole(struct q
                              stream->st,
                              args->flags);
     if (r == -1) {
+        remoteDispatchConnError(rerr, conn);
         virDomainFree(dom);
         remoteFreeClientStream(client, stream);
-        remoteDispatchConnError(rerr, conn);
         return -1;
     }
 
     if (remoteAddClientStream(client, stream, 1) < 0) {
-        virDomainFree(dom);
         remoteDispatchConnError(rerr, conn);
+        virDomainFree(dom);
         virStreamAbort(stream->st);
         remoteFreeClientStream(client, stream);
         return -1;