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.]
This commit is contained in:
Andrew Zaborowski 2018-08-13 17:21:50 +02:00 committed by Thomas Haller
parent 5ef8284a01
commit f2be625a07
2 changed files with 69 additions and 37 deletions

View file

@ -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",

View file

@ -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);