All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jordan Justen <jordan.l.justen@intel.com>
To: intel-gfx <intel-gfx@lists.freedesktop.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: [PATCH v3 4/4] drm/i915/guc: Verify hwconfig blob matches supported format
Date: Tue,  8 Feb 2022 13:05:03 -0800	[thread overview]
Message-ID: <20220208210503.869491-5-jordan.l.justen@intel.com> (raw)
In-Reply-To: <20220208210503.869491-1-jordan.l.justen@intel.com>

i915_drm.h now defines the format of the returned
DRM_I915_QUERY_HWCONFIG_BLOB query item. Since i915 receives this from
the black box GuC software, it should verify that the data matches
that format before sending it to user-space.

The verification makes a single simple pass through the blob contents,
so this verification step should not add a significant amount of init
time to i915.

v3:
 * Add various changes suggested by Tvrtko

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 56 ++++++++++++++++++-
 1 file changed, 53 insertions(+), 3 deletions(-)

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..350a0517b9f0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -71,7 +71,52 @@ static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
 	return 0;
 }
 
-static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
+static int verify_hwconfig_blob(struct drm_device *drm,
+				const struct intel_guc_hwconfig *hwconfig)
+{
+	struct drm_i915_query_hwconfig_blob_item *pos;
+	u32 remaining;
+
+	if (hwconfig->size % 4 != 0 || hwconfig->ptr == NULL)
+		return -EINVAL;
+
+	pos = hwconfig->ptr;
+	/* The number of dwords in the blob to validate. Each loop
+	 * pass will process at least 2 dwords corresponding to the
+	 * key and length fields of the item. In addition, the length
+	 * field of the item indicates the length of the data array,
+	 * and that number of dwords will be processed (skipped) as
+	 * well.
+	 */
+	remaining = hwconfig->size / 4;
+
+	while (remaining > 0) {
+		/* Each item requires at least 2 dwords for the key
+		 * and length fields. If the length field is 0, then
+		 * the data array would be of length 0.
+		 */
+		if (remaining < 2)
+			return -EINVAL;
+		/* remaining >= 2, so subtracting 2 is ok, whereas
+		 * adding 2 to pos->length could overflow.
+		 */
+		if (pos->length > remaining - 2)
+			return -EINVAL;
+		/* The length check above ensures that the adjustment
+		 * of the remaining variable will not underflow, and
+		 * that the adjustment of the pos variable will not
+		 * pass the end of the blob data.
+		 */
+		remaining -= 2 + pos->length;
+		pos = (void *)&pos->data[pos->length];
+	}
+
+	drm_dbg(drm, "hwconfig blob format is valid\n");
+	return 0;
+}
+
+static int guc_hwconfig_fill_buffer(struct drm_device *drm,
+				    struct intel_guc_hwconfig *hwconfig)
 {
 	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
 	struct i915_vma *vma;
@@ -88,8 +133,13 @@ static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
 	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
 
 	ret = __guc_action_get_hwconfig(hwconfig, ggtt_offset, hwconfig->size);
-	if (ret >= 0)
+	if (ret >= 0) {
 		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
+		if (verify_hwconfig_blob(drm, hwconfig)) {
+			drm_err(drm, "Ignoring invalid hwconfig blob received from GuC!\n");
+			ret = -EINVAL;
+		}
+	}
 
 	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
 
@@ -141,7 +191,7 @@ int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig)
 		return -ENOMEM;
 	}
 
-	ret = guc_hwconfig_fill_buffer(hwconfig);
+	ret = guc_hwconfig_fill_buffer(&i915->drm, hwconfig);
 	if (ret < 0) {
 		intel_guc_hwconfig_fini(hwconfig);
 		return ret;
-- 
2.34.1


WARNING: multiple messages have this Message-ID (diff)
From: Jordan Justen <jordan.l.justen@intel.com>
To: intel-gfx <intel-gfx@lists.freedesktop.org>
Cc: dri-devel <dri-devel@lists.freedesktop.org>
Subject: [Intel-gfx] [PATCH v3 4/4] drm/i915/guc: Verify hwconfig blob matches supported format
Date: Tue,  8 Feb 2022 13:05:03 -0800	[thread overview]
Message-ID: <20220208210503.869491-5-jordan.l.justen@intel.com> (raw)
In-Reply-To: <20220208210503.869491-1-jordan.l.justen@intel.com>

i915_drm.h now defines the format of the returned
DRM_I915_QUERY_HWCONFIG_BLOB query item. Since i915 receives this from
the black box GuC software, it should verify that the data matches
that format before sending it to user-space.

The verification makes a single simple pass through the blob contents,
so this verification step should not add a significant amount of init
time to i915.

v3:
 * Add various changes suggested by Tvrtko

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 56 ++++++++++++++++++-
 1 file changed, 53 insertions(+), 3 deletions(-)

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..350a0517b9f0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -71,7 +71,52 @@ static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
 	return 0;
 }
 
-static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
+static int verify_hwconfig_blob(struct drm_device *drm,
+				const struct intel_guc_hwconfig *hwconfig)
+{
+	struct drm_i915_query_hwconfig_blob_item *pos;
+	u32 remaining;
+
+	if (hwconfig->size % 4 != 0 || hwconfig->ptr == NULL)
+		return -EINVAL;
+
+	pos = hwconfig->ptr;
+	/* The number of dwords in the blob to validate. Each loop
+	 * pass will process at least 2 dwords corresponding to the
+	 * key and length fields of the item. In addition, the length
+	 * field of the item indicates the length of the data array,
+	 * and that number of dwords will be processed (skipped) as
+	 * well.
+	 */
+	remaining = hwconfig->size / 4;
+
+	while (remaining > 0) {
+		/* Each item requires at least 2 dwords for the key
+		 * and length fields. If the length field is 0, then
+		 * the data array would be of length 0.
+		 */
+		if (remaining < 2)
+			return -EINVAL;
+		/* remaining >= 2, so subtracting 2 is ok, whereas
+		 * adding 2 to pos->length could overflow.
+		 */
+		if (pos->length > remaining - 2)
+			return -EINVAL;
+		/* The length check above ensures that the adjustment
+		 * of the remaining variable will not underflow, and
+		 * that the adjustment of the pos variable will not
+		 * pass the end of the blob data.
+		 */
+		remaining -= 2 + pos->length;
+		pos = (void *)&pos->data[pos->length];
+	}
+
+	drm_dbg(drm, "hwconfig blob format is valid\n");
+	return 0;
+}
+
+static int guc_hwconfig_fill_buffer(struct drm_device *drm,
+				    struct intel_guc_hwconfig *hwconfig)
 {
 	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
 	struct i915_vma *vma;
@@ -88,8 +133,13 @@ static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
 	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
 
 	ret = __guc_action_get_hwconfig(hwconfig, ggtt_offset, hwconfig->size);
-	if (ret >= 0)
+	if (ret >= 0) {
 		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
+		if (verify_hwconfig_blob(drm, hwconfig)) {
+			drm_err(drm, "Ignoring invalid hwconfig blob received from GuC!\n");
+			ret = -EINVAL;
+		}
+	}
 
 	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
 
@@ -141,7 +191,7 @@ int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig)
 		return -ENOMEM;
 	}
 
-	ret = guc_hwconfig_fill_buffer(hwconfig);
+	ret = guc_hwconfig_fill_buffer(&i915->drm, hwconfig);
 	if (ret < 0) {
 		intel_guc_hwconfig_fini(hwconfig);
 		return ret;
-- 
2.34.1


  parent reply	other threads:[~2022-02-08 21:05 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-08 21:04 [PATCH v3 0/4] GuC HWCONFIG with documentation Jordan Justen
2022-02-08 21:04 ` [Intel-gfx] " Jordan Justen
2022-02-08 21:05 ` [PATCH v3 1/4] drm/i915/guc: Add fetch of hwconfig table Jordan Justen
2022-02-08 21:05   ` [Intel-gfx] " Jordan Justen
2022-02-08 21:56   ` Michal Wajdeczko
2022-02-08 21:56     ` [Intel-gfx] " Michal Wajdeczko
2022-02-08 21:05 ` [PATCH v3 2/4] drm/i915/uapi: Add query for hwconfig blob Jordan Justen
2022-02-08 21:05   ` [Intel-gfx] " Jordan Justen
2022-02-08 21:05 ` [PATCH v3 3/4] drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item Jordan Justen
2022-02-08 21:05   ` [Intel-gfx] " Jordan Justen
2022-02-08 21:05 ` Jordan Justen [this message]
2022-02-08 21:05   ` [Intel-gfx] [PATCH v3 4/4] drm/i915/guc: Verify hwconfig blob matches supported format Jordan Justen
2022-02-08 22:49   ` Michal Wajdeczko
2022-02-08 22:49     ` [Intel-gfx] " Michal Wajdeczko
2022-02-08 22:40 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for GuC HWCONFIG with documentation (rev3) Patchwork
2022-02-08 22:42 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-02-08 23:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-02-09  1:15 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-02-09 19:08 ` [Intel-gfx] [PATCH v3 0/4] GuC HWCONFIG with documentation Bloomfield, Jon
2022-02-09 19:08   ` Bloomfield, Jon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220208210503.869491-5-jordan.l.justen@intel.com \
    --to=jordan.l.justen@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.