[PATCH 2/2] remote: fix crash on OOM

[Eric Blake <eblake@xxxxxxxxxx>]

Bug introduced in commit 675464b.  On an OOM, this would try to
dereference a char* and free the contents as a pointer, which is
doomed to failure.

Adding a syntax check will prevent mistakes like this in the future.

* cfg.mk (sc_prohibit_internal_functions): New syntax check.
(exclude_file_name_regexp--sc_prohibit_internal_functions): Add
exemptions.
* daemon/remote.c (remoteRelayDomainEventIOError)
(remoteRelayDomainEventIOErrorReason)
(remoteRelayDomainEventGraphics, remoteRelayDomainEventBlockJob):
Use correct free function.
---
 cfg.mk          |   11 ++++++++++-
 daemon/remote.c |   28 ++++++++++++++--------------
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 95c5eff..9f4aa3e 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -212,7 +212,7 @@ useless_free_options =				\
 # y virDomainWatchdogDefFree
 # n virDrvNodeGetCellsFreeMemory (returns int)
 # n virDrvNodeGetFreeMemory (returns long long)
-# n virFree (dereferences param)
+# n virFree - dereferences param
 # n virFreeError
 # n virHashFree (takes 2 args)
 # y virInterfaceDefFree
@@ -306,6 +306,12 @@ sc_flags_usage:
 	halt='flags should be unsigned'					\
 	  $(_sc_search_regexp)

+# Avoid functions that should only be called via macro counterparts.
+sc_prohibit_internal_functions:
+	@prohibit='vir(Free|AllocN?|ReallocN|File(Close|Fclose|Fdopen)) *\(' \
+	halt='use VIR_ macros instead of internal functions'		\
+	  $(_sc_search_regexp)
+
 # Avoid functions that can lead to double-close bugs.
 sc_prohibit_close:
 	@prohibit='([^>.]|^)\<[fp]?close *\('				\
@@ -706,6 +712,9 @@ exclude_file_name_regexp--sc_prohibit_fork_wrappers = \

 exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/util\.c$$

+exclude_file_name_regexp--sc_prohibit_internal_functions = \
+  ^src/(util/(memory|util|virfile)\.[hc]|esx/esx_vi\.c)$$
+
 exclude_file_name_regexp--sc_prohibit_newline_at_end_of_diagnostic = \
   ^src/rpc/gendispatch\.pl$$

diff --git a/daemon/remote.c b/daemon/remote.c
index 74e759a..245d41c 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -250,8 +250,8 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED,
     return 0;
 mem_error:
     virReportOOMError();
-    virFree(data.srcPath);
-    virFree(data.devAlias);
+    VIR_FREE(data.srcPath);
+    VIR_FREE(data.devAlias);
     return -1;
 }

@@ -296,9 +296,9 @@ static int remoteRelayDomainEventIOErrorReason(virConnectPtr conn ATTRIBUTE_UNUS

 mem_error:
     virReportOOMError();
-    virFree(data.srcPath);
-    virFree(data.devAlias);
-    virFree(data.reason);
+    VIR_FREE(data.srcPath);
+    VIR_FREE(data.devAlias);
+    VIR_FREE(data.reason);
     return -1;
 }

@@ -374,17 +374,17 @@ static int remoteRelayDomainEventGraphics(virConnectPtr conn ATTRIBUTE_UNUSED,

 mem_error:
     virReportOOMError();
-    virFree(data.authScheme);
-    virFree(data.local.node);
-    virFree(data.local.service);
-    virFree(data.remote.node);
-    virFree(data.remote.service);
+    VIR_FREE(data.authScheme);
+    VIR_FREE(data.local.node);
+    VIR_FREE(data.local.service);
+    VIR_FREE(data.remote.node);
+    VIR_FREE(data.remote.service);
     if (data.subject.subject_val != NULL) {
         for (i = 0 ; i < data.subject.subject_len ; i++) {
-            virFree(data.subject.subject_val[i].type);
-            virFree(data.subject.subject_val[i].name);
+            VIR_FREE(data.subject.subject_val[i].type);
+            VIR_FREE(data.subject.subject_val[i].name);
         }
-        virFree(data.subject.subject_val);
+        VIR_FREE(data.subject.subject_val);
     }
     return -1;
 }
@@ -422,7 +422,7 @@ static int remoteRelayDomainEventBlockJob(virConnectPtr conn ATTRIBUTE_UNUSED,

 mem_error:
     virReportOOMError();
-    virFree(data.path);
+    VIR_FREE(data.path);
     return -1;
 }

-- 
1.7.4.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list