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@xxxxxxxxx>
---
.../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
