| Message ID | 1450110229-30450-10-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > We use MI_BATCH_BUFFER on 830/845, which means we need to know > the batch length. If the user passes a bad batch length (< 8) > the results is most likely a GPU hang. Rejct such batches. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Though, do you then remove the discrepancy in the next MI_BB_START patch? -Chris
On Mon, Dec 14, 2015 at 05:07:50PM +0000, Chris Wilson wrote: > On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > We use MI_BATCH_BUFFER on 830/845, which means we need to know > > the batch length. If the user passes a bad batch length (< 8) > > the results is most likely a GPU hang. Rejct such batches. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Though, do you then remove the discrepancy in the next MI_BB_START > patch? Yeah, I figured I'd still keep this in case the user passes in bogus batch_len, but now that I think about it, there's no way for values in the 1-7 range to reach us here due to the earlier 'batch_len & 7' check, and 0 would get replaced by the obj size. I guess we could drop this patch then.
On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > We use MI_BATCH_BUFFER on 830/845, which means we need to know > the batch length. If the user passes a bad batch length (< 8) > the results is most likely a GPU hang. Rejct such batches. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Can't we just reject a 0 batch_len everywhere? Well fix up igt first ofc. Slipping a 0 through really shouldn't be allowed. -Daniel > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 0c005a3dc57f..e878375be7eb 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1825,6 +1825,9 @@ i830_dispatch_execbuffer(struct drm_i915_gem_request *req, > u32 cs_offset = ring->scratch.gtt_offset; > int ret; > > + if (len < 8) > + return -EINVAL; > + > ret = intel_ring_begin(req, 6); > if (ret) > return ret; > -- > 2.4.10 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Dec 16, 2015 at 11:36:31AM +0100, Daniel Vetter wrote: > On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > We use MI_BATCH_BUFFER on 830/845, which means we need to know > > the batch length. If the user passes a bad batch length (< 8) > > the results is most likely a GPU hang. Rejct such batches. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Can't we just reject a 0 batch_len everywhere? Well fix up igt first ofc. > Slipping a 0 through really shouldn't be allowed. No, I sneaked that bit of ABI abuse passed you. Sorry. -Chris
On Wed, Dec 16, 2015 at 10:43:54AM +0000, Chris Wilson wrote: > On Wed, Dec 16, 2015 at 11:36:31AM +0100, Daniel Vetter wrote: > > On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > We use MI_BATCH_BUFFER on 830/845, which means we need to know > > > the batch length. If the user passes a bad batch length (< 8) > > > the results is most likely a GPU hang. Rejct such batches. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Can't we just reject a 0 batch_len everywhere? Well fix up igt first ofc. > > Slipping a 0 through really shouldn't be allowed. > > No, I sneaked that bit of ABI abuse passed you. Sorry. Hm, still should augment gem_exec_params to at least attempt ugly stuff like batch_len = 4 or batch_len = 3. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 0c005a3dc57f..e878375be7eb 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1825,6 +1825,9 @@ i830_dispatch_execbuffer(struct drm_i915_gem_request *req, u32 cs_offset = ring->scratch.gtt_offset; int ret; + if (len < 8) + return -EINVAL; + ret = intel_ring_begin(req, 6); if (ret) return ret;