From 0fd763cad42f36070b15ed8da4672acaac30ff5d Mon Sep 17 00:00:00 2001
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Date: Fri, 10 Aug 2018 08:43:44 +0300
Subject: [PATCH] thunderbolt: Use both locations for multi controller entry
 validation

Since the multi controller entry is part of the DROM and its place is
not fixed, it is possible (however unlikely) that the locations between
controller and the image are different. One scenario is that the new
image has typo in device or vendor name string fixed which could cause
the offset to be different.

To handle this case properly we need to read multi controller locations
of both controller and image separately, read their values and then
compare them against each other.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 plugins/thunderbolt/fu-thunderbolt-image.c | 79 ++++++++++++++++++----
 1 file changed, 66 insertions(+), 13 deletions(-)

diff --git a/plugins/thunderbolt/fu-thunderbolt-image.c b/plugins/thunderbolt/fu-thunderbolt-image.c
index 793d4b671..231248141 100644
--- a/plugins/thunderbolt/fu-thunderbolt-image.c
+++ b/plugins/thunderbolt/fu-thunderbolt-image.c
@@ -491,8 +491,66 @@ get_host_locations (guint16 id)
 	}
 }
 
+/*
+ * Finds optional multi controller (MC) entry from controller DROM.
+ * Returns TRUE if the controller did not have MC entry or the
+ * controller and image MC entries match. In any other case FALSE is
+ * returned and error is set accordingly.
+ */
+static gboolean
+compare_device_mc (const FuThunderboltFwObject *controller,
+		   const FuThunderboltFwObject *image,
+		   GError **error)
+{
+	FuThunderboltFwLocation image_mc_loc = { .section = DROM_SECTION, .description = "Multi Controller" };
+	FuThunderboltFwLocation controller_mc_loc = image_mc_loc;
+	g_autoptr(GByteArray) controller_mc = NULL;
+	g_autoptr(GByteArray) image_mc = NULL;
+
+	if (!read_drom_entry_location (controller, DROM_ENTRY_MC,
+				       &controller_mc_loc, error))
+		return FALSE;
+
+	/* it is fine if the controller does not have MC entry */
+	if (controller_mc_loc.offset == 0)
+		return TRUE;
+
+	if (!read_drom_entry_location (image, DROM_ENTRY_MC, &image_mc_loc, error))
+		return FALSE;
+
+	if (image_mc_loc.offset == 0) {
+		g_set_error_literal (error,
+				     FWUPD_ERROR, FWUPD_ERROR_INVALID_FILE,
+				     "firmware does not have multi controller entry");
+		return FALSE;
+	}
+	if (controller_mc_loc.len != image_mc_loc.len) {
+		g_set_error_literal (error,
+				     FWUPD_ERROR, FWUPD_ERROR_INVALID_FILE,
+				     "firmware multi controller entry length mismatch");
+		return FALSE;
+	}
+
+	controller_mc = read_location (&controller_mc_loc, controller, error);
+	if (controller_mc == NULL)
+		return FALSE;
+	image_mc = read_location (&image_mc_loc, image, error);
+	if (image_mc == NULL)
+		return FALSE;
+
+	if (memcmp (controller_mc->data, image_mc->data, controller_mc->len) != 0) {
+		g_set_error_literal (error,
+				     FWUPD_ERROR, FWUPD_ERROR_INVALID_FILE,
+				     "firmware multi controller entry mismatch");
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static const FuThunderboltFwLocation *
-get_device_locations (guint16 id, const FuThunderboltFwObject *fw, GError **error)
+get_device_locations (guint16 id, const FuThunderboltFwObject *controller,
+		      const FuThunderboltFwObject *image, GError **error)
 {
 	static const FuThunderboltFwLocation AR[] = {
 		{ .offset = 0x45,  .len = 1, .description = "Flash Size", .mask = 0x07 },
@@ -500,15 +558,11 @@ get_device_locations (guint16 id, const FuThunderboltFwObject *fw, GError **erro
 		{ 0 }
 	};
 
-	static FuThunderboltFwLocation TR[] = {
+	static const FuThunderboltFwLocation TR[] = {
 		{ .offset = 0x45,  .len = 1, .description = "Flash Size", .mask = 0x07 },
-		/* offset and len are read from DROM */
-		{ .section = DROM_SECTION, .description = "Multi Controller" },
 		{ 0 }
 	};
 
-	g_autoptr(GError) error_local = NULL;
-
 	switch (id) {
 	case 0x1578:
 	case 0x1576:
@@ -520,12 +574,10 @@ get_device_locations (guint16 id, const FuThunderboltFwObject *fw, GError **erro
 	case 0x15EA:
 	case 0x15EF:
 		/* if the controller has multi controller entry need to
-		 * compare it against the image. */
-		if (read_drom_entry_location (fw, DROM_ENTRY_MC, &TR[1],
-					      &error_local))
-			return TR;
-		g_debug ("failed to parse DROM entry: %s", error_local->message);
-		/* fallthrough */
+		 * compare it against the image first. */
+		if (!compare_device_mc (controller, image, error))
+			return NULL;
+		return TR;
 	default:
 		return NULL;
 	}
@@ -706,7 +758,8 @@ fu_plugin_thunderbolt_validate_image (GBytes  *controller_fw,
 			return VALIDATION_FAILED;
 		}
 	} else {
-		locations = get_device_locations (hw_info->id, &controller, error);
+		locations = get_device_locations (hw_info->id, &controller,
+						  &image, error);
 		if (locations == NULL) {
 			/* error is set already by the above */
 			return VALIDATION_FAILED;