Allow plugins to get DMI data from the hardware in a safe way

The returned strings are converted to ASCII and have leading and trailing
spaces removed.

plugins/uefi/fu-plugin-uefi.c

@@ -240,14 +240,15 @@ fu_plugin_update_offline (FuPlugin *plugin,
 }
 
 static AsVersionParseFlag
-fu_plugin_uefi_get_version_format (void)
+fu_plugin_uefi_get_version_format (FuPlugin *plugin)
 {
-	g_autofree gchar *content = NULL;
+	const gchar *content;
-	/* any vendors match */
+
-	if (!g_file_get_contents ("/sys/class/dmi/id/sys_vendor",
+	content = fu_plugin_get_dmi_value (plugin, FU_HWIDS_KEY_MANUFACTURER);
-				  &content, NULL, NULL))
+	if (content == NULL)
 		return AS_VERSION_PARSE_FLAG_USE_TRIPLET;
-	g_strchomp (content);
+
+	/* any vendors match */
 	for (guint i = 0; quirk_table[i].sys_vendor != NULL; i++) {
 		if (g_strcmp0 (content, quirk_table[i].sys_vendor) == 0)
 			return quirk_table[i].flags;
@@ -308,7 +309,7 @@ gboolean
 fu_plugin_coldplug (FuPlugin *plugin, GError **error)
 {
 	AsVersionParseFlag parse_flags;
-	g_autofree gchar *product_name = NULL;
+	const gchar *product_name;
 	fwup_resource *re;
 	gint supported;
 	g_autofree gchar *guid = NULL;
@@ -351,15 +352,11 @@ fu_plugin_coldplug (FuPlugin *plugin, GError **error)
 	}
 
 	/* set Display Name to the system for all capsules */
-	if (g_file_get_contents ("/sys/class/dmi/id/product_name",
+	product_name = fu_plugin_get_dmi_value (plugin, FU_HWIDS_KEY_PRODUCT_NAME);
-				 &product_name, NULL, NULL)) {
-		if (product_name != NULL)
-			g_strchomp (product_name);
-	}
 
 	/* add each device */
 	guid = g_strdup ("00000000-0000-0000-0000-000000000000");
-	parse_flags = fu_plugin_uefi_get_version_format ();
+	parse_flags = fu_plugin_uefi_get_version_format (plugin);
 	while (fwup_resource_iter_next (iter, &re) > 0) {
 		const gchar *uefi_type_str = NULL;
 		efi_guid_t *guid_raw;

src/fu-engine.c

@@ -63,7 +63,7 @@ struct _FuEngine
 	guint			 coldplug_delay;
 	GPtrArray		*plugins;	/* of FuPlugin */
 	GHashTable		*plugins_hash;	/* of name : FuPlugin */
-	GHashTable		*hwids;		/* of hwid : 1 */
+	FuHwids			*hwids;
 };
 
 enum {
@@ -2215,43 +2215,6 @@ fu_engine_plugin_set_coldplug_delay_cb (FuPlugin *plugin, guint duration, FuEngi
 		 duration, self->coldplug_delay);
 }
 
-#if AS_CHECK_VERSION(0,6,13)
-static gboolean
-fu_engine_load_hwids (FuEngine *self, GError **error)
-{
-	g_autoptr(FuHwids) hwids = fu_hwids_new ();
-
-	/* read files in /sys */
-	if (!fu_hwids_setup (hwids, NULL, error))
-		return FALSE;
-
-	/* add GUIDs */
-	for (guint i = 0; i < 15; i++) {
-		g_autofree gchar *guid = NULL;
-		g_autofree gchar *key = NULL;
-		g_autofree gchar *values = NULL;
-		g_autoptr(GError) error_local = NULL;
-
-		/* get the GUID and add to hash */
-		key = g_strdup_printf ("HardwareID-%u", i);
-		guid = fu_hwids_get_guid (hwids, key, &error_local);
-		if (guid == NULL) {
-			g_debug ("%s is not available, %s", key, error_local->message);
-			continue;
-		}
-		g_hash_table_insert (self->hwids,
-				     g_strdup (guid),
-				     GUINT_TO_POINTER (1));
-
-		/* show what makes up the GUID */
-		values = fu_hwids_get_replace_values (hwids, key, NULL);
-		g_debug ("{%s}   <- %s", guid, values);
-	}
-
-	return TRUE;
-}
-#endif
-
 static gint
 fu_engine_plugin_sort_cb (gconstpointer a, gconstpointer b)
 {
@@ -2424,13 +2387,11 @@ fu_engine_load (FuEngine *self, GError **error)
 				 G_USB_CONTEXT_FLAGS_AUTO_OPEN_DEVICES);
 #endif
 
-#if AS_CHECK_VERSION(0,6,13)
 	/* load the hwids */
-	if (!fu_engine_load_hwids (self, error)) {
+	if (!fu_hwids_setup (self->hwids, NULL, error)) {
 		g_prefix_error (error, "Failed to load hwids: ");
 		return FALSE;
 	}
-#endif
 
 	/* delete old data files */
 	if (!fu_engine_cleanup_state (error)) {
@@ -2505,7 +2466,7 @@ fu_engine_init (FuEngine *self)
 	self->status = FWUPD_STATUS_IDLE;
 	self->config = fu_config_new ();
 	self->devices = g_ptr_array_new_with_free_func ((GDestroyNotify) fu_engine_item_free);
-	self->hwids = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
+	self->hwids = fu_hwids_new ();
 	self->pending = fu_pending_new ();
 	self->profile = as_profile_new ();
 	self->store = as_store_new ();
@@ -2524,9 +2485,9 @@ fu_engine_finalize (GObject *obj)
 	if (self->coldplug_id != 0)
 		g_source_remove (self->coldplug_id);
 
-	g_hash_table_unref (self->hwids);
 	g_hash_table_unref (self->plugins_hash);
 	g_object_unref (self->config);
+	g_object_unref (self->hwids);
 	g_object_unref (self->pending);
 	g_object_unref (self->profile);
 	g_object_unref (self->store);

src/fu-hwids.c

@@ -30,15 +30,42 @@
 
 struct _FuHwids {
 	GObject			 parent_instance;
-	GHashTable		*hash;
+	GHashTable		*hash_dmi_hw;		/* BiosVersion->"1.2.3 " */
+	GHashTable		*hash_dmi_display;	/* BiosVersion->"1.2.3" */
+	GHashTable		*hash_guid;		/* a-c-b-d->1 */
 };
 
 G_DEFINE_TYPE (FuHwids, fu_hwids, G_TYPE_OBJECT)
 
+/**
+ * fu_hwids_get_value:
+ * @self: A #FuHwids
+ * @key: A DMI ID, e.g. "BiosVersion"
+ *
+ * Gets the cached value for one specific key that is valid ASCII and suitable
+ * for display.
+ *
+ * Returns: the string, e.g. "1.2.3", or %NULL if not found
+ **/
 const gchar *
 fu_hwids_get_value (FuHwids *self, const gchar *key)
 {
-	return g_hash_table_lookup (self->hash, key);
+	return g_hash_table_lookup (self->hash_dmi_display, key);
+}
+
+/**
+ * fu_hwids_has_guid:
+ * @self: A #FuHwids
+ * @guid: A GUID, e.g. "059eb22d-6dc7-59af-abd3-94bbe017f67c"
+ *
+ * Finds out if a hardware GUID exists.
+ *
+ * Returns: %TRUE if the GUID exists
+ **/
+gboolean
+fu_hwids_has_guid (FuHwids *self, const gchar *guid)
+{
+	return g_hash_table_lookup (self->hash_guid, guid) != NULL;
 }
 
 static gchar *
@@ -182,7 +209,7 @@ fu_hwids_get_replace_values (FuHwids *self, const gchar *keys, GError **error)
 	/* get each part of the HWID */
 	split = g_strsplit (keys, "&", -1);
 	for (guint j = 0; split[j] != NULL; j++) {
-		const gchar *tmp = g_hash_table_lookup (self->hash, split[j]);
+		const gchar *tmp = g_hash_table_lookup (self->hash_dmi_hw, split[j]);
 		if (tmp == NULL) {
 			g_set_error (error,
 				     G_IO_ERROR,
@@ -261,6 +288,7 @@ fu_hwids_setup (FuHwids *self, const gchar *sysfsdir, GError **error)
 	/* get all DMI data */
 	for (guint i = 0; sysfsfile[i].key != NULL; i++) {
 		g_autofree gchar *contents = NULL;
+		g_autofree gchar *contents_safe = NULL;
 		g_autofree gchar *fn = NULL;
 		const gchar *contents_hdr;
 
@@ -282,9 +310,39 @@ fu_hwids_setup (FuHwids *self, const gchar *sysfsdir, GError **error)
 		contents_hdr = contents;
 		while (contents_hdr[0] == '0')
 			contents_hdr++;
-		g_hash_table_insert (self->hash,
+		g_hash_table_insert (self->hash_dmi_hw,
 				     g_strdup (sysfsfile[i].key),
 				     g_strdup (contents_hdr));
+
+		/* make suitable for display */
+		contents_safe = g_str_to_ascii (contents_hdr, "C");
+		g_strchomp (contents_safe);
+		g_hash_table_insert (self->hash_dmi_display,
+				     g_strdup (sysfsfile[i].key),
+				     g_steal_pointer (&contents_safe));
+	}
+
+	/* add GUIDs */
+	for (guint i = 0; i < 15; i++) {
+		g_autofree gchar *guid = NULL;
+		g_autofree gchar *key = NULL;
+		g_autofree gchar *values = NULL;
+		g_autoptr(GError) error_local = NULL;
+
+		/* get the GUID and add to hash */
+		key = g_strdup_printf ("HardwareID-%u", i);
+		guid = fu_hwids_get_guid (self, key, &error_local);
+		if (guid == NULL) {
+			g_debug ("%s is not available, %s", key, error_local->message);
+			continue;
+		}
+		g_hash_table_insert (self->hash_guid,
+				     g_strdup (guid),
+				     GUINT_TO_POINTER (1));
+
+		/* show what makes up the GUID */
+		values = fu_hwids_get_replace_values (self, key, NULL);
+		g_debug ("{%s}   <- %s", guid, values);
 	}
 
 	return TRUE;
@@ -297,7 +355,9 @@ fu_hwids_finalize (GObject *object)
 	g_return_if_fail (FU_IS_HWIDS (object));
 	self = FU_HWIDS (object);
 
-	g_hash_table_unref (self->hash);
+	g_hash_table_unref (self->hash_dmi_hw);
+	g_hash_table_unref (self->hash_dmi_display);
+	g_hash_table_unref (self->hash_guid);
 	G_OBJECT_CLASS (fu_hwids_parent_class)->finalize (object);
 }
 
@@ -311,7 +371,9 @@ fu_hwids_class_init (FuHwidsClass *klass)
 static void
 fu_hwids_init (FuHwids *self)
 {
-	self->hash = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+	self->hash_dmi_hw = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+	self->hash_dmi_display = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+	self->hash_guid = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
 }
 
 FuHwids *

src/fu-hwids.h

@@ -54,6 +54,8 @@ gchar		*fu_hwids_get_replace_values	(FuHwids	*self,
 gchar		*fu_hwids_get_guid		(FuHwids	*self,
 						 const gchar	*keys,
 						 GError		**error);
+gboolean	 fu_hwids_has_guid		(FuHwids	*self,
+						 const gchar	*guid);
 gboolean	 fu_hwids_setup			(FuHwids	*self,
 						 const gchar	*sysfsdir,
 						 GError		**error);

src/fu-plugin-private.h

@@ -32,7 +32,7 @@ FuPlugin	*fu_plugin_new				(void);
 void		 fu_plugin_set_usb_context		(FuPlugin	*plugin,
 							 GUsbContext	*usb_ctx);
 void		 fu_plugin_set_hwids			(FuPlugin	*plugin,
-							 GHashTable	*hwids);
+							 FuHwids	*hwids);
 gboolean	 fu_plugin_open				(FuPlugin	*plugin,
 							 const gchar	*filename,
 							 GError		**error);

src/fu-plugin.c

@@ -43,7 +43,7 @@ typedef struct {
 	GUsbContext		*usb_ctx;
 	gboolean		 enabled;
 	gchar			*name;
-	GHashTable		*hwids;		/* hwid:1 */
+	FuHwids			*hwids;
 	GHashTable		*devices;	/* platform_id:GObject */
 	GHashTable		*devices_delay;	/* FuDevice:FuPluginHelper */
 	FuPluginData		*data;
@@ -490,7 +490,7 @@ fu_plugin_recoldplug (FuPlugin *plugin)
  * @plugin: A #FuPlugin
  * @hwid: A Hardware ID GUID, e.g. "6de5d951-d755-576b-bd09-c5cf66b27234"
  *
- * Checks to see if a specific hardware ID exists. All hardware IDs on a
+ * Checks to see if a specific GUID exists. All hardware IDs on a
  * specific system can be shown using the `fwupdmgr hwids` command.
  *
  * Since: 0.9.1
@@ -501,16 +501,32 @@ fu_plugin_check_hwid (FuPlugin *plugin, const gchar *hwid)
 	FuPluginPrivate *priv = GET_PRIVATE (plugin);
 	if (priv->hwids == NULL)
 		return FALSE;
-	return g_hash_table_lookup (priv->hwids, hwid) != NULL;
+	return fu_hwids_has_guid (priv->hwids, hwid);
+}
+
+/**
+ * fu_plugin_get_dmi_value:
+ * @plugin: A #FuPlugin
+ * @dmi_id: A DMI ID, e.g. "BiosVersion"
+ *
+ * Gets a hardware DMI value.
+ *
+ * Since: 0.9.7
+ **/
+const gchar *
+fu_plugin_get_dmi_value (FuPlugin *plugin, const gchar *dmi_id)
+{
+	FuPluginPrivate *priv = GET_PRIVATE (plugin);
+	if (priv->hwids == NULL)
+		return FALSE;
+	return fu_hwids_get_value (priv->hwids, dmi_id);
 }
 
 void
-fu_plugin_set_hwids (FuPlugin *plugin, GHashTable *hwids)
+fu_plugin_set_hwids (FuPlugin *plugin, FuHwids *hwids)
 {
 	FuPluginPrivate *priv = GET_PRIVATE (plugin);
-	if (priv->hwids != NULL)
+	g_set_object (&priv->hwids, hwids);
-		g_hash_table_unref (priv->hwids);
-	priv->hwids = g_hash_table_ref (hwids);
 }
 
 /**
@@ -1131,7 +1147,7 @@ fu_plugin_finalize (GObject *object)
 	if (priv->usb_ctx != NULL)
 		g_object_unref (priv->usb_ctx);
 	if (priv->hwids != NULL)
-		g_hash_table_unref (priv->hwids);
+		g_object_unref (priv->hwids);
 #ifndef RUNNING_ON_VALGRIND
 	if (priv->module != NULL)
 		g_module_close (priv->module);

src/fu-plugin.h

@@ -28,6 +28,7 @@
 #include <gusb.h>
 
 #include "fu-device.h"
+#include "fu-hwids.h"
 
 G_BEGIN_DECLS
 
@@ -92,6 +93,8 @@ void		 fu_plugin_cache_add			(FuPlugin	*plugin,
 							 gpointer	 dev);
 gboolean	 fu_plugin_check_hwid			(FuPlugin	*plugin,
 							 const gchar	*hwid);
+const gchar	*fu_plugin_get_dmi_value		(FuPlugin	*plugin,
+							 const gchar	*dmi_id);
 
 G_END_DECLS
 

src/fu-self-test.c

@@ -34,7 +34,6 @@
 #include "fu-hwids.h"
 #include "fu-test.h"
 
-#if AS_CHECK_VERSION(0,6,13)
 static void
 fu_hwids_func (void)
 {
@@ -44,6 +43,11 @@ fu_hwids_func (void)
 	g_autofree gchar *testdir = NULL;
 	gboolean ret;
 
+#if !AS_CHECK_VERSION(0,6,13)
+	g_test_skip ("appstreaml-glib too old, skipping");
+	return;
+#endif
+
 	struct {
 		const gchar *key;
 		const gchar *value;
@@ -94,8 +98,9 @@ fu_hwids_func (void)
 		g_assert_no_error (error);
 		g_assert_cmpstr (guid, ==, guids[i].value);
 	}
+	for (guint i = 0; guids[i].key != NULL; i++)
+		g_assert (fu_hwids_has_guid (hwids, guids[i].value));
 }
-#endif
 
 static void
 _plugin_status_changed_cb (FuPlugin *plugin, FwupdStatus status, gpointer user_data)
@@ -421,9 +426,7 @@ main (int argc, char **argv)
 	g_assert_cmpint (g_mkdir_with_parents ("/tmp/fwupd-self-test/var/lib/fwupd", 0755), ==, 0);
 
 	/* tests go here */
-#if AS_CHECK_VERSION(0,6,13)
 	g_test_add_func ("/fwupd/hwids", fu_hwids_func);
-#endif
 	g_test_add_func ("/fwupd/pending", fu_pending_func);
 	g_test_add_func ("/fwupd/plugin{delay}", fu_plugin_delay_func);
 	g_test_add_func ("/fwupd/plugin{module}", fu_plugin_module_func);