[PATCH] drm/i915/uc: Hardening firmware fetch

Michal Wajdeczko michal.wajdeczko at intel.com
Mon Aug 5 17:10:44 UTC 2019


Insert few more failure points into firmware fetch procedure
to check explicit fw disabling, using wrong blob name or
using mismatched or undefined fw versions.

While here, update messages (remove ptr, duplicated infos) and
stop treating all fetch errors as missing firmware case.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 141 ++++++++++++++++-------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h |   3 +
 2 files changed, 101 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index a3a22a26016c..8c74022cb8a7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -153,20 +153,61 @@ static const char *__override_huc_firmware_path(void)
 	return "";
 }
 
-static bool
-__uc_fw_override(struct intel_uc_fw *uc_fw)
+static void __uc_fw_user_override(struct intel_uc_fw *uc_fw)
 {
+	const char *path = NULL;
+
 	switch (uc_fw->type) {
 	case INTEL_UC_FW_TYPE_GUC:
-		uc_fw->path = __override_guc_firmware_path();
+		path = __override_guc_firmware_path();
 		break;
 	case INTEL_UC_FW_TYPE_HUC:
-		uc_fw->path = __override_huc_firmware_path();
+		path = __override_huc_firmware_path();
 		break;
 	}
 
-	uc_fw->user_overridden = uc_fw->path;
-	return uc_fw->user_overridden;
+	if (unlikely(path)) {
+		uc_fw->path = path;
+		uc_fw->user_overridden = true;
+	}
+}
+
+static void __uc_fw_fake_failures(struct intel_uc_fw *uc_fw,
+				  struct drm_i915_private *i915,
+				  bool user)
+{
+	if (i915_inject_probe_failure(i915)) {
+		/* disable */
+		uc_fw->path = "";
+		uc_fw->user_overridden = user;
+	} else if (i915_inject_probe_failure(i915)) {
+		/* non-existing blob */
+		uc_fw->path = intel_uc_fw_type_repr(uc_fw->type);
+		uc_fw->user_overridden = user;
+	} else if (i915_inject_probe_failure(i915)) {
+		/* newer major version */
+		uc_fw->major_ver_wanted += 1;
+		uc_fw->minor_ver_wanted = 0;
+		uc_fw->user_overridden = user;
+	} else if (i915_inject_probe_failure(i915)) {
+		/* newer minor version */
+		uc_fw->minor_ver_wanted += 1;
+		uc_fw->user_overridden = user;
+	} else if (uc_fw->major_ver_wanted && i915_inject_probe_failure(i915)) {
+		/* older major version */
+		uc_fw->major_ver_wanted -= 1;
+		uc_fw->minor_ver_wanted = 0;
+		uc_fw->user_overridden = user;
+	} else if (uc_fw->minor_ver_wanted && i915_inject_probe_failure(i915)) {
+		/* older minor version - this should always work! */
+		uc_fw->minor_ver_wanted -= 1;
+		uc_fw->user_overridden = user;
+	} else if (uc_fw->path && user && i915_inject_probe_failure(i915)) {
+		/* unsupported platform */
+		uc_fw->major_ver_wanted = 0;
+		uc_fw->minor_ver_wanted = 0;
+		uc_fw->user_overridden = user;
+	}
 }
 
 /**
@@ -192,9 +233,14 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 
 	uc_fw->type = type;
 
-	if (HAS_GT_UC(i915) && likely(!__uc_fw_override(uc_fw)))
+	if (HAS_GT_UC(i915)) {
 		__uc_fw_auto_select(uc_fw, INTEL_INFO(i915)->platform,
 				    INTEL_REVID(i915));
+		__uc_fw_user_override(uc_fw);
+
+		__uc_fw_fake_failures(uc_fw, i915, false);
+		__uc_fw_fake_failures(uc_fw, i915, true);
+	}
 
 	if (uc_fw->path && *uc_fw->path)
 		uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
@@ -220,17 +266,18 @@ void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 
 	GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
 
+	if (i915_inject_probe_failure(i915))
+		return;
+
 	err = request_firmware(&fw, uc_fw->path, i915->drm.dev);
 	if (err)
 		goto fail;
 
-	DRM_DEBUG_DRIVER("%s fw size %zu ptr %p\n",
-			 intel_uc_fw_type_repr(uc_fw->type), fw->size, fw);
-
 	/* Check the size of the blob before examining buffer contents */
-	if (fw->size < sizeof(struct uc_css_header)) {
-		DRM_WARN("%s: Unexpected firmware size (%zu, min %zu)\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
+	if (unlikely(fw->size < sizeof(struct uc_css_header))) {
+		dev_warn(i915->drm.dev,
+			 "Invalid %s firmware %s size: %zu < %zu\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
 			 fw->size, sizeof(struct uc_css_header));
 		err = -ENODATA;
 		goto fail;
@@ -241,10 +288,12 @@ void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	/* Check integrity of size values inside CSS header */
 	size = (css->header_size_dw - css->key_size_dw - css->modulus_size_dw -
 		css->exponent_size_dw) * sizeof(u32);
-	if (size != sizeof(struct uc_css_header)) {
-		DRM_WARN("%s: Mismatched firmware header definition\n",
-			 intel_uc_fw_type_repr(uc_fw->type));
-		err = -ENOEXEC;
+	if (unlikely(size != sizeof(struct uc_css_header))) {
+		dev_warn(i915->drm.dev,
+			 "Unexpected %s firmware %s header size: %zu != %zu\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			 fw->size, sizeof(struct uc_css_header));
+		err = -EPROTO;
 		goto fail;
 	}
 
@@ -252,19 +301,23 @@ void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
 
 	/* now RSA */
-	if (css->key_size_dw != UOS_RSA_SCRATCH_COUNT) {
-		DRM_WARN("%s: Mismatched firmware RSA key size (%u)\n",
-			 intel_uc_fw_type_repr(uc_fw->type), css->key_size_dw);
-		err = -ENOEXEC;
+	if (unlikely(css->key_size_dw != UOS_RSA_SCRATCH_COUNT)) {
+		dev_warn(i915->drm.dev,
+			 "Unexpected %s firmware %s RSA key size: %u != %u\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			 css->key_size_dw, UOS_RSA_SCRATCH_COUNT);
+		err = -EPROTO;
 		goto fail;
 	}
 	uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
 
 	/* At least, it should have header, uCode and RSA. Size of all three. */
 	size = sizeof(struct uc_css_header) + uc_fw->ucode_size + uc_fw->rsa_size;
-	if (fw->size < size) {
-		DRM_WARN("%s: Truncated firmware (%zu, expected %zu)\n",
-			 intel_uc_fw_type_repr(uc_fw->type), fw->size, size);
+	if (unlikely(fw->size < size)) {
+		dev_warn(i915->drm.dev,
+			 "Invalid %s firmware %s size: %zu < %zu\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			 fw->size, size);
 		err = -ENOEXEC;
 		goto fail;
 	}
@@ -290,32 +343,30 @@ void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 		break;
 	}
 
-	DRM_DEBUG_DRIVER("%s fw version %u.%u (wanted %u.%u)\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
-			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-
-	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
-		DRM_NOTE("%s: Skipping firmware version check\n",
-			 intel_uc_fw_type_repr(uc_fw->type));
-	} else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
-		   uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-		DRM_NOTE("%s: Wrong firmware version (%u.%u, required %u.%u)\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
+	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
+	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
+		dev_warn(i915->drm.dev,
+			 "Unexpected %s firmware %s version: %u.%u != %u.%u\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
 			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
 			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-		err = -ENOEXEC;
-		goto fail;
+		if (!intel_uc_fw_is_overridden(uc_fw)) {
+			err = -ENOEXEC;
+			goto fail;
+		}
 	}
 
 	obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
 	if (IS_ERR(obj)) {
 		err = PTR_ERR(obj);
-		DRM_DEBUG_DRIVER("%s fw object_create err=%d\n",
-				 intel_uc_fw_type_repr(uc_fw->type), err);
 		goto fail;
 	}
 
+	DRM_DEV_DEBUG_DRIVER(i915->drm.dev,
+			     "Fetched %s firmware: %s %u.%u\n",
+			     intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			     uc_fw->major_ver_found, uc_fw->minor_ver_found);
+
 	uc_fw->obj = obj;
 	uc_fw->size = fw->size;
 	uc_fw->status = INTEL_UC_FIRMWARE_AVAILABLE;
@@ -324,11 +375,15 @@ void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	return;
 
 fail:
-	uc_fw->status = INTEL_UC_FIRMWARE_MISSING;
+	if (err == -ENOENT)
+		uc_fw->status = INTEL_UC_FIRMWARE_MISSING;
+	else
+		uc_fw->status = INTEL_UC_FIRMWARE_ERROR;
 
-	DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
+	dev_info(i915->drm.dev, "Fetching %s%s firmware %s failed: error %d\n",
+		 intel_uc_fw_is_overridden(uc_fw) ? "non-default " : "",
 		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
-	DRM_INFO("%s: Firmware can be downloaded from %s\n",
+	dev_info(i915->drm.dev, "%s firmware(s) can be downloaded from %s\n",
 		 intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL);
 
 	release_firmware(fw);		/* OK even if fw is NULL */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index bfe3614613b7..db77ad2ef372 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -41,6 +41,7 @@ enum intel_uc_fw_status {
 	INTEL_UC_FIRMWARE_UNINITIALIZED = 0, /* used to catch checks done too early */
 	INTEL_UC_FIRMWARE_SELECTED, /* selected the blob we want to load */
 	INTEL_UC_FIRMWARE_MISSING, /* blob not found on the system */
+	INTEL_UC_FIRMWARE_ERROR, /* invalid format or version */
 	INTEL_UC_FIRMWARE_AVAILABLE, /* blob found and copied in mem */
 	INTEL_UC_FIRMWARE_FAIL, /* failed to xfer or init/auth the fw */
 	INTEL_UC_FIRMWARE_TRANSFERRED, /* dma xfer done */
@@ -91,6 +92,8 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 		return "SELECTED";
 	case INTEL_UC_FIRMWARE_MISSING:
 		return "MISSING";
+	case INTEL_UC_FIRMWARE_ERROR:
+		return "ERROR";
 	case INTEL_UC_FIRMWARE_AVAILABLE:
 		return "AVAILABLE";
 	case INTEL_UC_FIRMWARE_FAIL:
-- 
2.19.2



More information about the Intel-gfx-trybot mailing list