| Message ID | 20220222103640.1006006-5-jordan.l.justen@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | GuC HWCONFIG with documentation | expand |
On 22/02/2022 10:36, Jordan Justen wrote: > 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 > > v4: > * Rewrite verify_hwconfig_blob() to hopefully be clearer without > relying on comments so much, and add various suggestions from > Michal. > > Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> > Acked-by: Jon Bloomfield <jon.bloomfield@intel.com> > --- > .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c | 44 ++++++++++++++++++- > 1 file changed, 43 insertions(+), 1 deletion(-) > > 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 ad289603460c..a844b880cbdb 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c > @@ -73,9 +73,46 @@ static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig) > return 0; > } > > +static int verify_hwconfig_blob(struct intel_guc_hwconfig *hwconfig) > +{ > + struct intel_guc *guc = hwconfig_to_guc(hwconfig); > + struct drm_device *drm = &guc_to_gt(guc)->i915->drm; > + struct drm_i915_query_hwconfig_blob_item *item = hwconfig->ptr; > + u64 offset = 0; > + u64 remaining = hwconfig->size; > + /* Everything before the data field is required */ > + u64 min_item_size = offsetof(struct drm_i915_query_hwconfig_blob_item, data); > + u64 item_size; > + > + if (!IS_ALIGNED(hwconfig->size, sizeof(u32))) { > + drm_err(drm, "hwconfig blob size (%d) is not u32 aligned\n", hwconfig->size); > + return -EINVAL; > + } > + > + while (offset < hwconfig->size) { > + if (remaining < min_item_size) { > + drm_err(drm, "hwconfig blob invalid (no room for item required fields at offset %lld)\n", > + offset); > + return -EINVAL; > + } > + item_size = min_item_size + sizeof(u32) * item->length; > + if (item_size > remaining) { > + drm_err(drm, "hwconfig blob invalid (no room for data array of item at offset %lld)\n", > + offset); > + return -EINVAL; > + } > + offset += item_size; > + remaining -= item_size; > + item = (void *)&item->data[item->length]; > + } > + > + return 0; > +} > + > static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig) > { > struct intel_guc *guc = hwconfig_to_guc(hwconfig); > + struct drm_device *drm = &guc_to_gt(guc)->i915->drm; > struct i915_vma *vma; > u32 ggtt_offset; > void *vaddr; > @@ -90,8 +127,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(guc, ggtt_offset, hwconfig->size); > - if (ret >= 0) > + if (ret >= 0) { > memcpy(hwconfig->ptr, vaddr, hwconfig->size); > + if (verify_hwconfig_blob(hwconfig)) { > + drm_err(drm, "Ignoring invalid hwconfig blob received from GuC!\n"); > + ret = -EINVAL; > + } > + } > > i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP); > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
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 ad289603460c..a844b880cbdb 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c @@ -73,9 +73,46 @@ static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig) return 0; } +static int verify_hwconfig_blob(struct intel_guc_hwconfig *hwconfig) +{ + struct intel_guc *guc = hwconfig_to_guc(hwconfig); + struct drm_device *drm = &guc_to_gt(guc)->i915->drm; + struct drm_i915_query_hwconfig_blob_item *item = hwconfig->ptr; + u64 offset = 0; + u64 remaining = hwconfig->size; + /* Everything before the data field is required */ + u64 min_item_size = offsetof(struct drm_i915_query_hwconfig_blob_item, data); + u64 item_size; + + if (!IS_ALIGNED(hwconfig->size, sizeof(u32))) { + drm_err(drm, "hwconfig blob size (%d) is not u32 aligned\n", hwconfig->size); + return -EINVAL; + } + + while (offset < hwconfig->size) { + if (remaining < min_item_size) { + drm_err(drm, "hwconfig blob invalid (no room for item required fields at offset %lld)\n", + offset); + return -EINVAL; + } + item_size = min_item_size + sizeof(u32) * item->length; + if (item_size > remaining) { + drm_err(drm, "hwconfig blob invalid (no room for data array of item at offset %lld)\n", + offset); + return -EINVAL; + } + offset += item_size; + remaining -= item_size; + item = (void *)&item->data[item->length]; + } + + return 0; +} + static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig) { struct intel_guc *guc = hwconfig_to_guc(hwconfig); + struct drm_device *drm = &guc_to_gt(guc)->i915->drm; struct i915_vma *vma; u32 ggtt_offset; void *vaddr; @@ -90,8 +127,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(guc, ggtt_offset, hwconfig->size); - if (ret >= 0) + if (ret >= 0) { memcpy(hwconfig->ptr, vaddr, hwconfig->size); + if (verify_hwconfig_blob(hwconfig)) { + drm_err(drm, "Ignoring invalid hwconfig blob received from GuC!\n"); + ret = -EINVAL; + } + } i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);