Quoting Matthew Auld (2020-10-15 16:13:07)
> On 15/10/2020 12:59, Chris Wilson wrote:
> > Matthew Auld noted that on more recent systems (such as the parser for
> > gen9) we may have objects that are larger than expected by the GEM uAPI
> > (i.e. greater than u32). These objects would have incorrect implicit
> > batch lengths, causing the parser to reject them for being incomplete,
> > or worse.
> >
> > Based on a patch by Matthew Auld.
> >
> > Reported-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> > Fixes: 435e8fc059db ("drm/i915: Allow parsing of unsized batches")
> > Testcase: igt/gem_exec_params/larger-than-life-batch
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Matthew Auld <matthew.auld@xxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > Cc: Jon Bloomfield <jon.bloomfield@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index 4b09bcd70cf4..44b4558d5e86 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -287,8 +287,8 @@ struct i915_execbuffer {
> > u64 invalid_flags; /** Set of execobj.flags that are invalid */
> > u32 context_flags; /** Set of execobj.flags to insert from the ctx */
> >
> > + u64 batch_len; /** Length of batch within object */
> > u32 batch_start_offset; /** Location within object of batch */
> > - u32 batch_len; /** Length of batch within object */
> > u32 batch_flags; /** Flags composed for emit_bb_start() */
> > struct intel_gt_buffer_pool_node *batch_pool; /** pool node for batch buffer */
> >
> > @@ -871,6 +871,10 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
> >
> > if (eb->batch_len == 0)
> > eb->batch_len = eb->batch->vma->size - eb->batch_start_offset;
> > + if (eb->batch_len == 0) {
> > + drm_dbg(&i915->drm, "Invalid batch length\n");
> > + return -EINVAL;
> > + }
>
> This one should be impossible, or at least we should have hit the
> range_overflows check first?
It should be impossible, yes. I erred on the side of prudence; one little
check for the security conscious as the ramifications of it going wrong
are nasty.
Odd. I still feel in this instance, a check is better than a bug-on. I
must be unwell.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
