Quoting Tvrtko Ursulin (2017-11-16 09:44:20) > > On 16/11/2017 09:36, Chris Wilson wrote: > > 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. > > Oh right.. but looking at access_ok it seems that it is working with > unsigned longs. Not that it is documents explicitly, it is just a macro > and happens to use that under the covers (and via mm_segment_t type as > well). So it happens to match with size_t, but perhaps it indeed > warrants a comment and changing it to ULONG_MAX. GAH! size_t as you probably know was chosen for kmalloc_array. Ok, we are not using those functions here, so ulongs it is. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
