Re: [Intel-gfx] [PATCH 4/4] drm/i915/guc: Verify hwconfig blob matches supported format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi,

Commit message please.

Will GuC folks be reviewing this work?

Quick sanity check maybe makes sense, given data is being "sent" to userspace directly, I am just not sure if it is worth having in non-debug builds of i915. Though I will agree not having it in production then largely defeats the purpose so dunno. Effective difference if GuC load fails versus userspace libraries failing to parse hwconfig?

On 07/02/2022 19:28, Jordan Justen wrote:
Signed-off-by: Jordan Justen <jordan.l.justen@xxxxxxxxx>
---
  .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 26 +++++++++++++++++++
  1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
index ce6088f112d4..695ef7a8f519 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -71,6 +71,26 @@ static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
  	return 0;
  }
+static int verify_hwconfig_blob(const struct intel_guc_hwconfig *hwconfig)
+{
+	if (hwconfig->size % 4 != 0 || hwconfig->ptr == NULL)
+		return -EINVAL;

So individual item size is minimum one u32, or zero? Document that in patch 3?

+
+	struct drm_i915_query_hwconfig_blob_item *pos = hwconfig->ptr;

kbuild robot told you about mixing declarations and code already. :)

+	u32 remaining = (hwconfig->size / 4);

Blank line here and braces not needed.

+	while (remaining > 0) {
+		if (remaining < 2)
+			return -EINVAL;
+		if (pos->length > remaining - 2)
+			return -EINVAL;
+		remaining -= 2 + pos->length;
+		pos = (void *)&pos->data[pos->length];
+	}
+
+	DRM_INFO("hwconfig blob format appears valid\n");

Probably debug level at most.

+	return 0;
+}
+
  static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
  {
  	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
@@ -91,6 +111,12 @@ static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
  	if (ret >= 0)
  		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
+ if (verify_hwconfig_blob(hwconfig)) {

Merge under the "ret >= 0" branch above?

+		DRM_ERROR("Ignoring invalid hwconfig blob received from "
+			  "GuC!\n");

Use drm_dbg/drm_err so the log is tied to a device in multi-gpu systems.

Also keep the string on one line as per kernel coding style guide.

Regards,

Tvrtko

+		return -EINVAL;
+	}
+
  	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
return ret;



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux