Quoting Chris Wilson (2020-10-13 12:58:31)
> Quoting Matthew Auld (2020-10-13 12:18:39)
> > As per the ABI batch_len is u32, however if the batch_len is left unset,
> > then the kernel will just assume batch_len is the size of the whole
> > batch object, however since the vma->size is u64, while the batch_len is
> > just u32 we can end up with batch_len = 0 if we are given too large batch
> > object(e.g 1ULL << 32), which doesn't look the intended behaviour and
> > probably leads to explosions on some HW.
> >
> > Testcase: igt/gem_exec_params/larger-than-life-batch
> > Fixes: 0b5372727be3 ("drm/i915/cmdparser: Use cached vmappings")
>
> Nah. That's setting exec_len used for dispatch, not for parsing, which
> is still using
>
> i915_gem_execbuffer_parse(engine, &shadow_exec_entry,
> params->batch->obj,
> eb,
> args->batch_start_offset,
> args->batch_len,
> drm_is_current_master(file));
> (and args->batch_len is straight from userspace and passed onwards)
>
> It's right up until 435e8fc059db ("drm/i915: Allow parsing of unsized batches")
> where we are using the user value of batch_len for allocating the shadow
> object and parsing.
>
> Fixes: 435e8fc059db ("drm/i915: Allow parsing of unsized batches")
>
> > Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index 4b09bcd70cf4..80c738c72e6e 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -869,8 +869,13 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
> > return -EINVAL;
> > }
> >
> > - if (eb->batch_len == 0)
> > + if (eb->batch_len == 0) {
> > eb->batch_len = eb->batch->vma->size - eb->batch_start_offset;
>
> if (overflows_type(eb->batch->vma->size - eb->batch_start_offset, eb->batch_len))
And we shouldn't fail here. So bump to u64, and since the vma->size
cannot be larger than the GTT, we will be safe for gen2 where it makes a
difference.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
