dbus: Don't unref NULL messages

Apparently we are not the only ones with dumb free functions
because dbus_message_unref() does not accept NULL either. But if
I were to vote, this one is even more evil. Instead of returning
an error just like we do it immediately dereference any pointer
passed and thus crash you app. Well done DBus!

  Program received signal SIGSEGV, Segmentation fault.
  [Switching to Thread 0x7f878ebda700 (LWP 31264)]
  0x00007f87be4016e5 in ?? () from /usr/lib64/libdbus-1.so.3
  (gdb) bt
  #0  0x00007f87be4016e5 in ?? () from /usr/lib64/libdbus-1.so.3
  #1  0x00007f87be3f004e in dbus_message_unref () from /usr/lib64/libdbus-1.so.3
  #2  0x00007f87bf6ecf95 in virSystemdGetMachineNameByPID (pid=9849) at util/virsystemd.c:228
  #3  0x00007f879761bd4d in qemuConnectCgroup (driver=0x7f87600a32a0, vm=0x7f87600c7550) at qemu/qemu_cgroup.c:909
  #4  0x00007f87976386b7 in qemuProcessReconnect (opaque=0x7f87600db840) at qemu/qemu_process.c:3386
  #5  0x00007f87bf6edfff in virThreadHelper (data=0x7f87600d5580) at util/virthread.c:206
  #6  0x00007f87bb602334 in start_thread (arg=0x7f878ebda700) at pthread_create.c:333
  #7  0x00007f87bb3481bd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
  (gdb) frame 2
  #2  0x00007f87bf6ecf95 in virSystemdGetMachineNameByPID (pid=9849) at util/virsystemd.c:228
  228         dbus_message_unref(reply);
  (gdb) p reply
  $1 = (DBusMessage *) 0x0

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5ae3618ce4a4264be23426d5ea8c16919bb41a39..4cfaed53190fb46496f5d5069fd1b96cc8ed4d2c 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1370,6 +1370,7 @@ virDBusHasSystemBus;
 virDBusMessageDecode;
 virDBusMessageEncode;
 virDBusMessageRead;
+virDBusMessageUnref;
 virDBusSetSharedBus;
 
 

diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 910f2662079bd42ee432e58be41997a97d98b3bc..18c962c1da0a809c98613d9f628cdb7f96bd04a4 100644 (file)
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -374,7 +374,7 @@ virNetDaemonGotInhibitReply(DBusPendingCall *pending,
             VIR_FORCE_CLOSE(fd);
         }
     }
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
 
  cleanup:
     virObjectUnlock(dmn);
@@ -426,7 +426,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn,
                                      dmn, NULL);
         dmn->autoShutdownCallingInhibit = true;
     }
-    dbus_message_unref(message);
+    virDBusMessageUnref(message);
 }
 #endif
 

diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 78fb7953dca2e6ae9716b95a516ddadf1e73f36b..3f4dbe34a7a1bfd71fd117e6882138268a7ff2c4 100644 (file)
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1398,7 +1398,7 @@ int virDBusCreateMethodV(DBusMessage **call,
     }
 
     if (virDBusMessageEncodeArgs(*call, types, args) < 0) {
-        dbus_message_unref(*call);
+        virDBusMessageUnref(*call);
         *call = NULL;
         goto cleanup;
     }
@@ -1467,7 +1467,7 @@ int virDBusCreateReplyV(DBusMessage **reply,
     }
 
     if (virDBusMessageEncodeArgs(*reply, types, args) < 0) {
-        dbus_message_unref(*reply);
+        virDBusMessageUnref(*reply);
         *reply = NULL;
         goto cleanup;
     }
@@ -1586,7 +1586,7 @@ virDBusCall(DBusConnection *conn,
         if (ret == 0 && replyout)
             *replyout = reply;
         else
-            dbus_message_unref(reply);
+            virDBusMessageUnref(reply);
     }
     return ret;
 }
@@ -1650,8 +1650,7 @@ int virDBusCallMethod(DBusConnection *conn,
     ret = virDBusCall(conn, call, replyout, error);
 
  cleanup:
-    if (call)
-        dbus_message_unref(call);
+    virDBusMessageUnref(call);
     return ret;
 }
 
@@ -1727,7 +1726,7 @@ static int virDBusIsServiceInList(const char *listMethod, const char *name)
     }
 
  cleanup:
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     return ret;
 }
 
@@ -1763,7 +1762,8 @@ int virDBusIsServiceRegistered(const char *name)
 
 void virDBusMessageUnref(DBusMessage *msg)
 {
-    dbus_message_unref(msg);
+    if (msg)
+        dbus_message_unref(msg);
 }
 
 #else /* ! WITH_DBUS */

diff --git a/src/util/virdbus.h b/src/util/virdbus.h
index 9e86538d5a7a3ac050d9ede7d3add5892181ca79..86b4223ffb4be47f6ad15564747b8b2662a46312 100644 (file)
--- a/src/util/virdbus.h
+++ b/src/util/virdbus.h
@@ -28,7 +28,6 @@
 # else
 #  define DBusConnection void
 #  define DBusMessage void
-#  define dbus_message_unref(m) do {} while (0)
 # endif
 # include "internal.h"
 

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index a972c05d5baba3859dd5705939dd0bd655f61066..f26fd865c97bb2c595b962ae510f5795e0531bbc 100644 (file)
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -822,8 +822,7 @@ virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
 
  cleanup:
     virResetError(&error);
-    if (reply)
-        dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     return ret;
 }
 

diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 667779809920617d41f3b5ba9998c648820b8c34..4883f94920da383edcdfa13a274b3833864f777c 100644 (file)
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -225,7 +225,7 @@ virSystemdGetMachineNameByPID(pid_t pid)
 
  cleanup:
     VIR_FREE(object);
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
 
     return name;
 }

diff --git a/tests/virdbustest.c b/tests/virdbustest.c
index 4ec3c0ddbdaac884de4897766d07ed5d15f0a1e6..1622b031158e2f466ec129e9d5db6bad11964757 100644 (file)
--- a/tests/virdbustest.c
+++ b/tests/virdbustest.c
@@ -121,7 +121,7 @@ static int testMessageSimple(const void *args ATTRIBUTE_UNUSED)
     VIR_FREE(out_string);
     VIR_FREE(out_signature);
     VIR_FREE(out_objectpath);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -171,7 +171,7 @@ static int testMessageVariant(const void *args ATTRIBUTE_UNUSED)
  cleanup:
     VIR_FREE(out_str1);
     VIR_FREE(out_str2);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -230,7 +230,7 @@ static int testMessageArray(const void *args ATTRIBUTE_UNUSED)
  cleanup:
     VIR_FREE(out_str1);
     VIR_FREE(out_str2);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -274,7 +274,7 @@ static int testMessageEmptyArrayRef(const void *args ATTRIBUTE_UNUSED)
     ret = 0;
 
  cleanup:
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -323,7 +323,7 @@ static int testMessageSingleArrayRef(const void *args ATTRIBUTE_UNUSED)
  cleanup:
     if (out_strv1)
         VIR_FREE(out_strv1[0]);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -436,7 +436,7 @@ static int testMessageArrayRef(const void *args ATTRIBUTE_UNUSED)
     for (i = 0; i < out_nstrv2; i++)
         VIR_FREE(out_strv2[i]);
     VIR_FREE(out_strv2);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -511,7 +511,7 @@ static int testMessageStruct(const void *args ATTRIBUTE_UNUSED)
     VIR_FREE(out_string);
     VIR_FREE(out_signature);
     VIR_FREE(out_objectpath);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -581,7 +581,7 @@ static int testMessageDict(const void *args ATTRIBUTE_UNUSED)
     VIR_FREE(out_key1);
     VIR_FREE(out_key2);
     VIR_FREE(out_key3);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -652,7 +652,7 @@ static int testMessageDictRef(const void *args ATTRIBUTE_UNUSED)
         VIR_FREE(out_strv1[5]);
     }
     VIR_FREE(out_strv1);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -695,7 +695,7 @@ static int testMessageEmptyDictRef(const void *args ATTRIBUTE_UNUSED)
     ret = 0;
 
  cleanup:
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 

diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
index 1f8d8f1e9fa474cc549449bd8ec28883f9a7247c..8f6fc9e12d26d6a745505ba3a74d275e0107de67 100644 (file)
--- a/tests/virfirewalltest.c
+++ b/tests/virfirewalltest.c
@@ -179,8 +179,7 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
     return reply;
 
  error:
-    if (reply)
-        dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     reply = NULL;
     if (error && !dbus_error_is_set(error))
         dbus_set_error_const(error,

diff --git a/tests/virpolkittest.c b/tests/virpolkittest.c
index cdf78f56d7b916e0797263b51d7b630cb756dca0..b39beedf14183908bbed17381a98a67e484efea3 100644 (file)
--- a/tests/virpolkittest.c
+++ b/tests/virpolkittest.c
@@ -140,7 +140,7 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
     return reply;
 
  error:
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     return NULL;
 }
 

diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 46452dd7436a083c03b4f3f9249345ec31626740..101f5e049fe010d80731287f04468414830ecb6d 100644 (file)
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -28,6 +28,7 @@
 # include <dbus/dbus.h>
 
 # include "virsystemd.h"
+# include "virdbus.h"
 # include "virlog.h"
 # include "virmock.h"
 # define VIR_FROM_THIS VIR_FROM_NONE
@@ -151,7 +152,7 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
     return reply;
 
  error:
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     return NULL;
 }