[PATCH 1/5] lib: Don't unref message passed to virGDBusCallMethod{WithFD}()

The virGDBusCallMethod() and virGDBusCallMethodWithFD() are simple wrappers over g_dbus_connection_call_sync() and g_dbus_connection_call_with_unix_fd_list_sync() respectively. The documentation to these function states that passed parameters (@message in our case) is consumed for 'convenient' inline use of g_variant_new() [1]. But that means we must not unref the message afterwards. To make it explicit that the message is consumed the signature of our wrappers is changed too. 1: https://developer.gnome.org/gio/stable/GDBusConnection.html#g-dbus-connec... Reported-by: Cole Robinson <crobinso(a)redhat.com&gt; Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/rpc/virnetdaemon.c | 4 ++-- src/util/virfirewalld.c | 16 ++++++++-------- src/util/virgdbus.c | 29 ++++++++++++++++++++--------- src/util/virgdbus.h | 4 ++-- src/util/virpolkit.c | 4 ++-- src/util/virsystemd.c | 23 ++++++++--------------- 6 files changed, 42 insertions(+), 38 deletions(-) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 37a5662e04..b42feb7f1e 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -469,7 +469,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn, { g_autoptr(GVariant) reply = NULL; g_autoptr(GUnixFDList) replyFD = NULL; - g_autoptr(GVariant) message = NULL; + GVariant *message = NULL; GDBusConnection *systemBus; int fd; int rc; @@ -494,7 +494,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn, "/org/freedesktop/login1", "org.freedesktop.login1.Manager", "Inhibit", - message, + &message, NULL); if (rc < 0) diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c index a94ac7c183..b00f3bf418 100644 --- a/src/util/virfirewalld.c +++ b/src/util/virfirewalld.c @@ -83,7 +83,7 @@ int virFirewallDGetVersion(unsigned long *version) { GDBusConnection *sysbus = virGDBusGetSystemBus(); - g_autoptr(GVariant) message = NULL; + GVariant *message = NULL; g_autoptr(GVariant) reply = NULL; g_autoptr(GVariant) gvar = NULL; char *versionStr; @@ -101,7 +101,7 @@ virFirewallDGetVersion(unsigned long *version) "/org/fedoraproject/FirewallD1", "org.freedesktop.DBus.Properties", "Get", - message) < 0) + &message) < 0) return -1; g_variant_get(reply, "(v)", &gvar); @@ -129,7 +129,7 @@ int virFirewallDGetBackend(void) { GDBusConnection *sysbus = virGDBusGetSystemBus(); - g_autoptr(GVariant) message = NULL; + GVariant *message = NULL; g_autoptr(GVariant) reply = NULL; g_autoptr(GVariant) gvar = NULL; g_autoptr(virError) error = NULL; @@ -154,7 +154,7 @@ virFirewallDGetBackend(void) "/org/fedoraproject/FirewallD1/config", "org.freedesktop.DBus.Properties", "Get", - message) < 0) + &message) < 0) return -1; if (error->level == VIR_ERR_ERROR) { @@ -273,7 +273,7 @@ virFirewallDApplyRule(virFirewallLayer layer, { const char *ipv = virFirewallLayerFirewallDTypeToString(layer); GDBusConnection *sysbus = virGDBusGetSystemBus(); - g_autoptr(GVariant) message = NULL; + GVariant *message = NULL; g_autoptr(GVariant) reply = NULL; g_autoptr(virError) error = NULL; @@ -304,7 +304,7 @@ virFirewallDApplyRule(virFirewallLayer layer, "/org/fedoraproject/FirewallD1", "org.fedoraproject.FirewallD1.direct", "passthrough", - message) < 0) + &message) < 0) return -1; if (error->level == VIR_ERR_ERROR) { @@ -353,7 +353,7 @@ virFirewallDInterfaceSetZone(const char *iface, const char *zone) { GDBusConnection *sysbus = virGDBusGetSystemBus(); - g_autoptr(GVariant) message = NULL; + GVariant *message = NULL; if (!sysbus) return -1; @@ -368,5 +368,5 @@ virFirewallDInterfaceSetZone(const char *iface, "/org/fedoraproject/FirewallD1", "org.fedoraproject.FirewallD1.zone", "changeZoneOfInterface", - message); + &message); } diff --git a/src/util/virgdbus.c b/src/util/virgdbus.c index 4360a6acff..856ad80a88 100644 --- a/src/util/virgdbus.c +++ b/src/util/virgdbus.c @@ -205,23 +205,26 @@ virGDBusCallMethod(GDBusConnection *conn, const char *objectPath, const char *ifaceName, const char *method, - GVariant *data) + GVariant **data) { g_autoptr(GVariant) ret = NULL; g_autoptr(GError) gerror = NULL; + GVariant *parameters = NULL; if (error) memset(error, 0, sizeof(*error)); - if (data) - g_variant_ref_sink(data); + if (data && *data) { + parameters = g_variant_ref_sink(*data); + *data = NULL; + } ret = g_dbus_connection_call_sync(conn, busName, objectPath, ifaceName, method, - data, + parameters, replyType, G_DBUS_CALL_FLAGS_NONE, VIR_DBUS_METHOD_CALL_TIMEOUT_MILIS, @@ -259,24 +262,27 @@ virGDBusCallMethodWithFD(GDBusConnection *conn, const char *objectPath, const char *ifaceName, const char *method, - GVariant *data, + GVariant **data, GUnixFDList *dataFD) { g_autoptr(GVariant) ret = NULL; g_autoptr(GError) gerror = NULL; + GVariant *parameters = NULL; if (error) memset(error, 0, sizeof(*error)); - if (data) - g_variant_ref_sink(data); + if (data && *data) { + parameters = g_variant_ref_sink(*data); + *data = NULL; + } ret = g_dbus_connection_call_with_unix_fd_list_sync(conn, busName, objectPath, ifaceName, method, - data, + parameters, replyType, G_DBUS_CALL_FLAGS_NONE, VIR_DBUS_METHOD_CALL_TIMEOUT_MILIS, @@ -317,9 +323,14 @@ virGDBusCallMethodWithFD(GDBusConnection *conn G_GNUC_UNUSED, const char *objectPath G_GNUC_UNUSED, const char *ifaceName G_GNUC_UNUSED, const char *method G_GNUC_UNUSED, - GVariant *data G_GNUC_UNUSED, + GVariant **data, GUnixFDList *dataFD G_GNUC_UNUSED) { + if (data && *data) { + g_variant_unref(*data); + *data = NULL; + } + virReportSystemError(ENOSYS, "%s", _("Unix file descriptors not supported on this platform")); return -1; diff --git a/src/util/virgdbus.h b/src/util/virgdbus.h index ca7073e27c..9c3955073c 100644 --- a/src/util/virgdbus.h +++ b/src/util/virgdbus.h @@ -51,7 +51,7 @@ virGDBusCallMethod(GDBusConnection *conn, const char *objectPath, const char *ifaceName, const char *method, - GVariant *data); + GVariant **data); int virGDBusCallMethodWithFD(GDBusConnection *conn, @@ -63,7 +63,7 @@ virGDBusCallMethodWithFD(GDBusConnection *conn, const char *objectPath, const char *ifaceName, const char *method, - GVariant *data, + GVariant **data, GUnixFDList *dataFD); int diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c index aad924a065..e9fb4ec64f 100644 --- a/src/util/virpolkit.c +++ b/src/util/virpolkit.c @@ -67,7 +67,7 @@ int virPolkitCheckAuth(const char *actionid, GVariantBuilder builder; GVariant *gprocess = NULL; GVariant *gdetails = NULL; - g_autoptr(GVariant) message = NULL; + GVariant *message = NULL; g_autoptr(GVariant) reply = NULL; g_autoptr(GVariantIter) iter = NULL; char *retkey; @@ -110,7 +110,7 @@ int virPolkitCheckAuth(const char *actionid, "/org/freedesktop/PolicyKit1/Authority", "org.freedesktop.PolicyKit1.Authority", "CheckAuthorization", - message) < 0) + &message) < 0) return -1; g_variant_get(reply, "((bba{ss}))", &is_authorized, &is_challenge, &iter); diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 8456085476..f849cbbf60 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -198,7 +198,7 @@ char * virSystemdGetMachineNameByPID(pid_t pid) { GDBusConnection *conn; - g_autoptr(GVariant) message = NULL; + GVariant *message = NULL; g_autoptr(GVariant) reply = NULL; g_autoptr(GVariant) gvar = NULL; g_autofree char *object = NULL; @@ -220,7 +220,7 @@ virSystemdGetMachineNameByPID(pid_t pid) "/org/freedesktop/machine1", "org.freedesktop.machine1.Manager", "GetMachineByPID", - message) < 0) + &message) < 0) return NULL; g_variant_get(reply, "(o)", &object); @@ -231,7 +231,6 @@ virSystemdGetMachineNameByPID(pid_t pid) VIR_DEBUG("Domain with pid %lld has object path '%s'", (long long) pid, object); - g_variant_unref(message); message = g_variant_new("(ss)", "org.freedesktop.machine1.Machine", "Name"); @@ -243,7 +242,7 @@ virSystemdGetMachineNameByPID(pid_t pid) object, "org.freedesktop.DBus.Properties", "Get", - message) < 0) + &message) < 0) return NULL; g_variant_get(reply, "(v)", &gvar); @@ -393,9 +392,7 @@ int virSystemdCreateMachine(const char *name, "/org/freedesktop/machine1", "org.freedesktop.machine1.Manager", "CreateMachineWithNetwork", - message); - - g_variant_unref(message); + &message); if (rc < 0) return -1; @@ -440,9 +437,7 @@ int virSystemdCreateMachine(const char *name, "/org/freedesktop/machine1", "org.freedesktop.machine1.Manager", "CreateMachine", - message); - - g_variant_unref(message); + &message); if (rc < 0) return -1; @@ -468,9 +463,7 @@ int virSystemdCreateMachine(const char *name, "/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "SetUnitProperties", - message); - - g_variant_unref(message); + &message); if (rc < 0) return -1; @@ -483,7 +476,7 @@ int virSystemdTerminateMachine(const char *name) { int rc; GDBusConnection *conn; - g_autoptr(GVariant) message = NULL; + GVariant *message = NULL; g_autoptr(virError) error = NULL; if (!name) @@ -519,7 +512,7 @@ int virSystemdTerminateMachine(const char *name) "/org/freedesktop/machine1", "org.freedesktop.machine1.Manager", "TerminateMachine", - message) < 0) + &message) < 0) return -1; if (error->level == VIR_ERR_ERROR && -- 2.26.2