From 8ef139faba949f563cfb5acb15c43903cf660552 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Tue, 23 Apr 2019 19:33:28 +0100 Subject: [PATCH] Only use class-based instance IDs for quirk matching These are dangerous GUIDs that shouldn't generally be used by any device for distribution of firmware updates. They'll potentially apply to devices outside of the vendor they were created for. Additionally if two USB devices with common interface classes are on the system, they may lead to one device not being populated by fwupd. Fixes https://github.com/hughsie/fwupd/issues/1162 --- src/fu-device-private.h | 9 +++++++++ src/fu-device.c | 39 ++++++++++++++++++++++++++------------- src/fu-udev-device.c | 4 +++- src/fu-usb-device.c | 13 +++++++++---- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/fu-device-private.h b/src/fu-device-private.h index 02b1d0ecc..b78b67ed7 100644 --- a/src/fu-device-private.h +++ b/src/fu-device-private.h @@ -11,6 +11,12 @@ G_BEGIN_DECLS +typedef enum { + FU_DEVICE_INSTANCE_FLAG_NONE = 0, + FU_DEVICE_INSTANCE_FLAG_ONLY_QUIRKS = 1 << 0, + FU_DEVICE_INSTANCE_FLAG_LAST +} FuDeviceInstanceFlags; + GPtrArray *fu_device_get_parent_guids (FuDevice *self); gboolean fu_device_has_parent_guid (FuDevice *self, const gchar *guid); @@ -29,5 +35,8 @@ gboolean fu_device_ensure_id (FuDevice *self, void fu_device_incorporate_from_component (FuDevice *device, XbNode *component); void fu_device_convert_instance_ids (FuDevice *self); +void fu_device_add_instance_id_full (FuDevice *self, + const gchar *instance_id, + FuDeviceInstanceFlags flags); G_END_DECLS diff --git a/src/fu-device.c b/src/fu-device.c index ada83bcb9..2d624ae1e 100644 --- a/src/fu-device.c +++ b/src/fu-device.c @@ -860,6 +860,29 @@ fu_device_has_guid (FuDevice *self, const gchar *guid) return fwupd_device_has_guid (FWUPD_DEVICE (self), guid); } +/* private */ +void +fu_device_add_instance_id_full (FuDevice *self, + const gchar *instance_id, + FuDeviceInstanceFlags flags) +{ + g_autofree gchar *guid = NULL; + if (fwupd_guid_is_valid (instance_id)) { + g_warning ("use fu_device_add_guid(\"%s\") instead!", instance_id); + fu_device_add_guid_safe (self, instance_id); + return; + } + + /* it seems odd adding the instance ID and the GUID quirks and not just + * calling fu_device_add_guid_safe() -- but we want the quirks to match + * so the plugin is set, but not the LVFS metadata to match firmware + * until we're sure the device isn't using _NO_AUTO_INSTANCE_IDS */ + guid = fwupd_guid_hash_string (instance_id); + fu_device_add_guid_quirks (self, guid); + if ((flags & FU_DEVICE_INSTANCE_FLAG_ONLY_QUIRKS) == 0) + fwupd_device_add_instance_id (FWUPD_DEVICE (self), instance_id); +} + /** * fu_device_add_instance_id: * @self: A #FuDevice @@ -873,19 +896,9 @@ fu_device_has_guid (FuDevice *self, const gchar *guid) void fu_device_add_instance_id (FuDevice *self, const gchar *instance_id) { - g_autofree gchar *guid = NULL; - if (fwupd_guid_is_valid (instance_id)) { - g_warning ("use fu_device_add_guid(\"%s\") instead!", instance_id); - fu_device_add_guid_safe (self, instance_id); - return; - } - /* it seems odd adding the instance ID and the GUID quirks and not just - * calling fu_device_add_guid_safe() -- but we want the quirks to match - * so the plugin is set, but not the LVFS metadata to match firmware - * until we're sure the device isn't using _NO_AUTO_INSTANCE_IDS */ - guid = fwupd_guid_hash_string (instance_id); - fu_device_add_guid_quirks (self, guid); - fwupd_device_add_instance_id (FWUPD_DEVICE (self), instance_id); + g_return_if_fail (FU_IS_DEVICE (self)); + g_return_if_fail (instance_id != NULL); + fu_device_add_instance_id_full (self, instance_id, FU_DEVICE_INSTANCE_FLAG_NONE); } /** diff --git a/src/fu-udev-device.c b/src/fu-udev-device.c index da8e10d40..f81e298c2 100644 --- a/src/fu-udev-device.c +++ b/src/fu-udev-device.c @@ -10,6 +10,7 @@ #include +#include "fu-device-private.h" #include "fu-udev-device-private.h" /** @@ -225,7 +226,8 @@ fu_udev_device_probe (FuDevice *device, GError **error) if (priv->vendor != 0x0000) { g_autofree gchar *devid = NULL; devid = g_strdup_printf ("%s\\VEN_%04X", subsystem, priv->vendor); - fu_device_add_instance_id (device, devid); + fu_device_add_instance_id_full (device, devid, + FU_DEVICE_INSTANCE_FLAG_ONLY_QUIRKS); } /* subclassed */ diff --git a/src/fu-usb-device.c b/src/fu-usb-device.c index b19c34591..e25cfec20 100644 --- a/src/fu-usb-device.c +++ b/src/fu-usb-device.c @@ -8,6 +8,7 @@ #include "config.h" +#include "fu-device-private.h" #include "fu-usb-device-private.h" /** @@ -254,7 +255,8 @@ fu_usb_device_probe (FuDevice *device, GError **error) fu_device_add_instance_id (device, devid1); devid0 = g_strdup_printf ("USB\\VID_%04X", g_usb_device_get_vid (priv->usb_device)); - fu_device_add_instance_id (device, devid0); + fu_device_add_instance_id_full (device, devid0, + FU_DEVICE_INSTANCE_FLAG_ONLY_QUIRKS); /* add the interface GUIDs */ intfs = g_usb_device_get_interfaces (priv->usb_device, error); @@ -269,14 +271,17 @@ fu_usb_device_probe (FuDevice *device, GError **error) g_usb_interface_get_class (intf), g_usb_interface_get_subclass (intf), g_usb_interface_get_protocol (intf)); - fu_device_add_instance_id (device, intid1); + fu_device_add_instance_id_full (device, intid1, + FU_DEVICE_INSTANCE_FLAG_ONLY_QUIRKS); intid2 = g_strdup_printf ("USB\\CLASS_%02X&SUBCLASS_%02X", g_usb_interface_get_class (intf), g_usb_interface_get_subclass (intf)); - fu_device_add_instance_id (device, intid2); + fu_device_add_instance_id_full (device, intid2, + FU_DEVICE_INSTANCE_FLAG_ONLY_QUIRKS); intid3 = g_strdup_printf ("USB\\CLASS_%02X", g_usb_interface_get_class (intf)); - fu_device_add_instance_id (device, intid3); + fu_device_add_instance_id_full (device, intid3, + FU_DEVICE_INSTANCE_FLAG_ONLY_QUIRKS); } /* subclassed */