From f2be625a07191c4256de0ce2ffbaad404dfe7c99 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Mon, 13 Aug 2018 17:21:50 +0200 Subject: [PATCH] wifi/iwd: Check g_dbus_proxy_get_cached_property return values Instead of passing the return values to g_variant_get_string or g_variant_boolean and then checking the return value of that call, add wrappers that first check's whether the variant is non-NULL and of the right type. g_variant_get_string doesn't allow a NULL parameter and will also never return NULL according to the docs. For the State property we assume a state "unknown" and emit a warning if the property can't be read, "unknown" is also a string in IWD itself which would be returned if something went really wrong. In any case this shouldn't happen. [thaller@redhat.com: fix missing initialization of nm_auto() variable interfaces.] --- src/devices/wifi/nm-device-iwd.c | 41 +++++++++++++++---- src/devices/wifi/nm-iwd-manager.c | 65 +++++++++++++++++-------------- 2 files changed, 69 insertions(+), 37 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 3be6159f24..b880340cac 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -772,6 +772,32 @@ complete_connection (NMDevice *device, return TRUE; } +static gboolean +get_variant_boolean (GVariant *v, const char *property) +{ + if (!v || !g_variant_is_of_type (v, G_VARIANT_TYPE_BOOLEAN)) { + nm_log_warn (LOGD_DEVICE | LOGD_WIFI, + "Property %s not cached or not boolean type", property); + + return FALSE; + } + + return g_variant_get_boolean (v); +} + +static const char * +get_variant_state (GVariant *v) +{ + if (!v || !g_variant_is_of_type (v, G_VARIANT_TYPE_STRING)) { + nm_log_warn (LOGD_DEVICE | LOGD_WIFI, + "State property not cached or not a string"); + + return "unknown"; + } + + return g_variant_get_string (v, NULL); +} + static gboolean is_available (NMDevice *device, NMDeviceCheckDevAvailableFlags flags) { @@ -783,7 +809,7 @@ is_available (NMDevice *device, NMDeviceCheckDevAvailableFlags flags) return FALSE; value = g_dbus_proxy_get_cached_property (priv->dbus_proxy, "Powered"); - return g_variant_get_boolean (value); + return get_variant_boolean (value, "Powered"); } static gboolean @@ -1756,7 +1782,8 @@ state_changed (NMDeviceIwd *self, const char *new_state) NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT); return; - } + } else if (nm_streq (new_state, "unknown")) + return; _LOGE (LOGD_WIFI, "State %s unknown", new_state); } @@ -1802,13 +1829,13 @@ properties_changed (GDBusProxy *proxy, GVariant *changed_properties, g_variant_get (changed_properties, "a{sv}", &iter); while (g_variant_iter_next (iter, "{&sv}", &key, &value)) { if (!strcmp (key, "State")) - state_changed (self, g_variant_get_string (value, NULL)); + state_changed (self, get_variant_state (value)); if (!strcmp (key, "Scanning")) - scanning_changed (self, g_variant_get_boolean (value)); + scanning_changed (self, get_variant_boolean (value, "Scanning")); if (!strcmp (key, "Powered")) - powered_changed (self, g_variant_get_boolean (value)); + powered_changed (self, get_variant_boolean (value, "Powered")); g_variant_unref (value); } @@ -1849,12 +1876,12 @@ nm_device_iwd_set_dbus_object (NMDeviceIwd *self, GDBusObject *object) priv->dbus_proxy = G_DBUS_PROXY (interface); value = g_dbus_proxy_get_cached_property (priv->dbus_proxy, "Scanning"); - priv->scanning = g_variant_get_boolean (value); + priv->scanning = get_variant_boolean (value, "Scanning"); g_variant_unref (value); priv->scan_requested = FALSE; value = g_dbus_proxy_get_cached_property (priv->dbus_proxy, "State"); - state_changed (self, g_variant_get_string (value, NULL)); + state_changed (self, get_variant_state (value)); g_variant_unref (value); g_signal_connect (priv->dbus_proxy, "g-properties-changed", diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 37210c99ef..05b8416375 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -83,6 +83,32 @@ G_DEFINE_TYPE (NMIwdManager, nm_iwd_manager, G_TYPE_OBJECT) /*****************************************************************************/ +static const char * +get_variant_string_or_null (GVariant *v) +{ + if (!v) + return NULL; + + if ( !g_variant_is_of_type (v, G_VARIANT_TYPE_STRING) + && !g_variant_is_of_type (v, G_VARIANT_TYPE_OBJECT_PATH)) + return NULL; + + return g_variant_get_string (v, NULL); +} + +static const char * +get_property_string_or_null (GDBusProxy *proxy, const char *property) +{ + gs_unref_variant GVariant *value = NULL; + + if (!proxy || !property) + return NULL; + + value = g_dbus_proxy_get_cached_property (proxy, property); + + return get_variant_string_or_null (value); +} + static void agent_dbus_method_cb (GDBusConnection *connection, const char *sender, const char *object_path, @@ -95,7 +121,6 @@ agent_dbus_method_cb (GDBusConnection *connection, NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); const char *network_path, *device_path, *ifname; gs_unref_object GDBusInterface *network = NULL, *device_obj = NULL; - gs_unref_variant GVariant *value = NULL; int ifindex; NMDevice *device; gs_free char *name_owner = NULL; @@ -113,9 +138,8 @@ agent_dbus_method_cb (GDBusConnection *connection, network = g_dbus_object_manager_get_interface (priv->object_manager, network_path, NM_IWD_NETWORK_INTERFACE); - value = g_dbus_proxy_get_cached_property (G_DBUS_PROXY (network), "Device"); - device_path = g_variant_get_string (value, NULL); + device_path = get_property_string_or_null (G_DBUS_PROXY (network), "Device"); if (!device_path) { _LOGD ("agent-request: device not cached for network %s in IWD Agent request", network_path); @@ -125,10 +149,8 @@ agent_dbus_method_cb (GDBusConnection *connection, device_obj = g_dbus_object_manager_get_interface (priv->object_manager, device_path, NM_IWD_DEVICE_INTERFACE); - g_variant_unref (value); - value = g_dbus_proxy_get_cached_property (G_DBUS_PROXY (device_obj), "Name"); - ifname = g_variant_get_string (value, NULL); + ifname = get_property_string_or_null (G_DBUS_PROXY (device_obj), "Name"); if (!ifname) { _LOGD ("agent-request: name not cached for device %s in IWD Agent request", device_path); @@ -257,7 +279,6 @@ set_device_dbus_object (NMIwdManager *self, GDBusInterface *interface, { NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); GDBusProxy *proxy; - GVariant *value; const char *ifname; int ifindex; NMDevice *device; @@ -273,16 +294,14 @@ set_device_dbus_object (NMIwdManager *self, GDBusInterface *interface, NM_IWD_DEVICE_INTERFACE)) return; - value = g_dbus_proxy_get_cached_property (proxy, "Name"); - if (!value) { + ifname = get_property_string_or_null (proxy, "Name"); + if (!ifname) { _LOGE ("Name not cached for Device at %s", g_dbus_proxy_get_object_path (proxy)); return; } - ifname = g_variant_get_string (value, NULL); ifindex = if_nametoindex (ifname); - g_variant_unref (value); if (!ifindex) { _LOGE ("if_nametoindex failed for Name %s for Device at %s: %i", @@ -388,10 +407,10 @@ list_known_networks_cb (GObject *source, GAsyncResult *res, gpointer user_data) while (g_variant_iter_next (props, "{&sv}", &key, &val)) { if (!strcmp (key, "Name")) - name = g_variant_get_string (val, NULL); + name = get_variant_string_or_null (val); if (!strcmp (key, "Type")) - type = g_variant_get_string (val, NULL); + type = get_variant_string_or_null (val); g_variant_unref (val); } @@ -492,28 +511,14 @@ device_added (NMManager *manager, NMDevice *device, gpointer user_data) objects = g_dbus_object_manager_get_objects (priv->object_manager); for (iter = objects; iter; iter = iter->next) { GDBusObject *object = G_DBUS_OBJECT (iter->data); - GDBusInterface *interface; - GDBusProxy *proxy; - GVariant *value; + gs_unref_object GDBusInterface *interface = NULL; const char *obj_ifname; interface = g_dbus_object_get_interface (object, NM_IWD_DEVICE_INTERFACE); - if (!interface) - continue; + obj_ifname = get_property_string_or_null ((GDBusProxy *) interface, "Name"); - proxy = G_DBUS_PROXY (interface); - value = g_dbus_proxy_get_cached_property (proxy, "Name"); - if (!value) { - g_object_unref (interface); - continue; - } - - obj_ifname = g_variant_get_string (value, NULL); - g_variant_unref (value); - g_object_unref (interface); - - if (strcmp (nm_device_get_iface (device), obj_ifname)) + if (!obj_ifname || strcmp (nm_device_get_iface (device), obj_ifname)) continue; nm_device_iwd_set_dbus_object (NM_DEVICE_IWD (device), object);