From: Kees Cook
Sent: 06 December 2017 20:29
There's no good reason to separate the access_ok() from the copy, especially since the access_ok() size is hard-coded instead of using sizeof(). Instead, just use copy_from_user() directly.
Looks like an optimisation to save doing the access_ok() check for every 'fence'.
OTOH 'user copy hardening' probably makes a much larger performance degradation on this kind of code. (Might be ok here because &fence probably refers to something in the current stack frame.)
David
Fixes: cf6e7bac6357 ("drm/i915: Add support for drm syncobjs") Cc: Jason Ekstrand jason@jlekstrand.net Cc: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Kees Cook keescook@chromium.org
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 435ed95df144..1da703213b17 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2087,8 +2087,6 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args, return ERR_PTR(-EINVAL);
user = u64_to_user_ptr(args->cliprects_ptr);
if (!access_ok(VERIFY_READ, user, nfences * 2 * sizeof(u32)))
return ERR_PTR(-EFAULT);fences = kvmalloc_array(args->num_cliprects, sizeof(*fences), __GFP_NOWARN | GFP_KERNEL);
@@ -2099,7 +2097,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args, struct drm_i915_gem_exec_fence fence; struct drm_syncobj *syncobj;
if (__copy_from_user(&fence, user++, sizeof(fence))) {
if (copy_from_user(&fence, user++, sizeof(fence))) { err = -EFAULT; goto err;-- 2.7.4
-- Kees Cook Pixel Security