From 862298a2e7bef059b73f477e8a88d403c523e10b Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Thu, 11 Feb 2016 11:14:11 +0100 Subject: [PATCH] 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 --- src/libvirt_private.syms | 1 + src/rpc/virnetdaemon.c | 4 ++-- src/util/virdbus.c | 14 +++++++------- src/util/virdbus.h | 1 - src/util/virfirewall.c | 3 +-- src/util/virsystemd.c | 2 +- tests/virdbustest.c | 20 ++++++++++---------- tests/virfirewalltest.c | 3 +-- tests/virpolkittest.c | 2 +- tests/virsystemdtest.c | 3 ++- 10 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5ae3618ce4..4cfaed5319 100644 --- 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 910f266207..18c962c1da 100644 --- 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 78fb7953dc..3f4dbe34a7 100644 --- 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 9e86538d5a..86b4223ffb 100644 --- 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 a972c05d5b..f26fd865c9 100644 --- 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 6677798099..4883f94920 100644 --- 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 4ec3c0ddbd..1622b03115 100644 --- 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 1f8d8f1e9f..8f6fc9e12d 100644 --- 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 cdf78f56d7..b39beedf14 100644 --- 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 46452dd743..101f5e049f 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -28,6 +28,7 @@ # include # 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; } -- 2.39.5