[Intel-gfx] [PATCH] drm/i915/gem: Support parsing of oversize batches

[Chris Wilson]

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 at intel.com>
> > Fixes: 435e8fc059db ("drm/i915: Allow parsing of unsized batches")
> > Testcase: igt/gem_exec_params/larger-than-life-batch
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.auld at intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Cc: Jon Bloomfield <jon.bloomfield at intel.com>
> > ---
> >   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