Re: [PATCH] drm/i915: Avoid undefined behaviour of "u32 >> 32"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 29/06/2017 14:49, Chris Wilson wrote:
When computing a hash for looking up relcoation target handles in an
execbuf, we start with a large size for the hashtable and proceed to
reduce it until the allocation suceeds. The final attempt is with an
order of 0 (i.e. a single element). This means that we then pass bits=0
to hash_32() which then computes "hash >> (32 - 0)" to lookup the single
element. Right shifting by a value the width of the operand is
undefined, so limit the smallest hash table we use to order 1.

Fixes: 4ff4b44cbb70 ("drm/i915: Store a direct lookup from object handle to vma")
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 22 +++++++++++-----------
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 718bb75ad387..54791bcb8ccb 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -296,12 +296,8 @@ static int eb_create(struct i915_execbuffer *eb)
  				break;
  		} while (--size);
- if (unlikely(!eb->buckets)) {
-			eb->buckets = kzalloc(sizeof(struct hlist_head),
-					      GFP_TEMPORARY);

Want to maybe drop the NORETRY | NOWARN from the remaining allocation now? Wasn't it recently discussed that it is to feeble in it's attempts to allocate?

-			if (unlikely(!eb->buckets))
-				return -ENOMEM;
-		}
+		if (unlikely(!eb->buckets))
+			return -ENOMEM;
eb->lut_size = size;
  	} else {
@@ -453,7 +449,7 @@ eb_add_vma(struct i915_execbuffer *eb,
  			return err;
  	}
- if (eb->lut_size >= 0) {
+	if (eb->lut_size > 0) {
  		vma->exec_handle = entry->handle;
  		hlist_add_head(&vma->exec_node,
  			       &eb->buckets[hash_32(entry->handle,
@@ -897,7 +893,7 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
  static void eb_reset_vmas(const struct i915_execbuffer *eb)
  {
  	eb_release_vmas(eb);
-	if (eb->lut_size >= 0)
+	if (eb->lut_size > 0)
  		memset(eb->buckets, 0,
  		       sizeof(struct hlist_head) << eb->lut_size);
  }
@@ -906,7 +902,7 @@ static void eb_destroy(const struct i915_execbuffer *eb)
  {
  	GEM_BUG_ON(eb->reloc_cache.rq);
- if (eb->lut_size >= 0)
+	if (eb->lut_size > 0)
  		kfree(eb->buckets);
  }
@@ -2185,8 +2181,11 @@ i915_gem_do_execbuffer(struct drm_device *dev,
  		}
  	}
- if (eb_create(&eb))
-		return -ENOMEM;
+	err = eb_create(&eb);
+	if (err)
+		goto err_out_fence;
+
+	GEM_BUG_ON(!eb.lut_size);
err = eb_select_context(&eb);
  	if (unlikely(err))
@@ -2346,6 +2345,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
  	i915_gem_context_put(eb.ctx);
  err_destroy:
  	eb_destroy(&eb);
+err_out_fence:
  	if (out_fence_fd != -1)
  		put_unused_fd(out_fence_fd);
  err_in_fence:


Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux