Quoting Tvrtko Ursulin (2017-11-16 09:00:08) > > On 15/11/2017 16:08, Chris Wilson wrote: > > @@ -2083,14 +2083,14 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args, > > if (!(args->flags & I915_EXEC_FENCE_ARRAY)) > > return NULL; > > > > - if (nfences > SIZE_MAX / sizeof(*fences)) > > + if (nfences > SIZE_MAX / max(sizeof(*fences), sizeof(*user))) > > return ERR_PTR(-EINVAL); > > This check is just to stay in some reasonable limits, yes? Doesn't seem > to be directly related to the implementation below. I am not saying we > should allow more fences, just wondering whether it is worthy of a > comment? And maybe in that case it is not -EINVAL (ABI) but -ENOMEM (to > fake implementation cannot support it / doesn't want to support it)? > Assuming I did not miss something. > > user = u64_to_user_ptr(args->cliprects_ptr); > > - if (!access_ok(VERIFY_READ, user, nfences * 2 * sizeof(u32))) > > + if (!access_ok(VERIFY_READ, user, nfences * sizeof(*user))) > > return ERR_PTR(-EFAULT); > > For access_ok we use nfences * sizeof(*user), therefore we have to make sure that doesn't overflow and produce small value which then passes the check and allows the caller to wander off into the badlands. So really the question is it EFAULT for the invalid pointer or EINVAL or the unusable parameters. We've (I've) taken the stance that we wanted to differentiate between the parameters being bogus and the result of using those parameters being bogus. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
