On Thu, 19 Sep 2013, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > There's actually no real risk since we already check for stricter > constraints earlier (using UINT_MAX / sizeof (struct > drm_i915_gem_exec_object2) as the limit). But in eb_create we use > signed integers, which steals a factor of 2. Luckily struct > drm_i915_gem_exec_object2 for this to not matter. Parse error. > Still, be consistent and use unsigned integers. > > Similar use unsinged integers when checking for overflows in the > relocation entry processing. > > I've also added a new subtests to igt/gem_reloc_overflow to also > test for overflowing args->buffer_count values. > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index a733118..cd98aeb 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -45,18 +45,19 @@ struct eb_vmas { > static struct eb_vmas * > eb_create(struct drm_i915_gem_execbuffer2 *args, struct i915_address_space *vm) > { > + unsigned size, count; My OCD says always use "unsigned int", not just "unsigned". To me this is like having: const foo; static bar; YMMV. Otherwise, Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> > struct eb_vmas *eb = NULL; > > if (args->flags & I915_EXEC_HANDLE_LUT) { > - int size = args->buffer_count; > + size = args->buffer_count; > size *= sizeof(struct i915_vma *); > size += sizeof(struct eb_vmas); > eb = kmalloc(size, GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); > } > > if (eb == NULL) { > - int size = args->buffer_count; > - int count = PAGE_SIZE / sizeof(struct hlist_head) / 2; > + size = args->buffer_count; > + count = PAGE_SIZE / sizeof(struct hlist_head) / 2; > BUILD_BUG_ON_NOT_POWER_OF_2(PAGE_SIZE / sizeof(struct hlist_head)); > while (count > 2*size) > count >>= 1; > @@ -667,7 +668,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev, > bool need_relocs; > int *reloc_offset; > int i, total, ret; > - int count = args->buffer_count; > + unsigned count = args->buffer_count; > > if (WARN_ON(list_empty(&eb->vmas))) > return 0; > @@ -818,8 +819,8 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec, > int count) > { > int i; > - int relocs_total = 0; > - int relocs_max = INT_MAX / sizeof(struct drm_i915_gem_relocation_entry); > + unsigned relocs_total = 0; > + unsigned relocs_max = UINT_MAX / sizeof(struct drm_i915_gem_relocation_entry); > > for (i = 0; i < count; i++) { > char __user *ptr = to_user_ptr(exec[i].relocs_ptr); > -- > 1.8.4.rc3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx