From 06c99161b66d36b0345c443bd0934cfc3f4d7f54 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 20 Jan 2014 19:52:29 +0100 Subject: [PATCH 1/6] drm/udl: fix error-path when damage-req fails We need to call dma_buf_end_cpu_access() in case a damage-request. Unlikely, but might happen during device unplug. Reviewed-by: Daniel Vetter Signed-off-by: David Herrmann --- drivers/gpu/drm/udl/udl_fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index dbadd49e4c4a..377176372da8 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -421,7 +421,7 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, clips[i].x2 - clips[i].x1, clips[i].y2 - clips[i].y1); if (ret) - goto unlock; + break; } if (ufb->obj->base.import_attach) { From 2b932d8ef009f37d397c211b1dc5d0b056f6ef64 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 20 Jan 2014 19:54:18 +0100 Subject: [PATCH 2/6] drm/udl: fix Bpp calculation in dumb_create() Probably a typo.. we obviously need "(bpp + 7) / 8" instead of "(bpp + 1) / 8". Unlikely to be hit in any sane code, but lets be safe. Use DIV_ROUND_UP() to avoid the problem entirely and make the core more readable. Reviewed-by: Daniel Vetter Signed-off-by: David Herrmann --- drivers/gpu/drm/udl/udl_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c index 8d67b943ac05..be4fcd0f0e0f 100644 --- a/drivers/gpu/drm/udl/udl_gem.c +++ b/drivers/gpu/drm/udl/udl_gem.c @@ -60,7 +60,7 @@ int udl_dumb_create(struct drm_file *file, struct drm_device *dev, struct drm_mode_create_dumb *args) { - args->pitch = args->width * ((args->bpp + 1) / 8); + args->pitch = args->width * DIV_ROUND_UP(args->bpp, 8); args->size = args->pitch * args->height; return udl_gem_create(file, dev, args->size, &args->handle); From 16d2831d6f590681ef239562ac6d73c605e7d6dc Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 20 Jan 2014 20:07:49 +0100 Subject: [PATCH 3/6] drm/gem: fix indentation Remove double-whitespace and wrong indentation. Signed-off-by: David Herrmann --- drivers/gpu/drm/drm_gem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 5bbad873c798..dd8e38a22e23 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -692,7 +692,7 @@ drm_gem_object_release(struct drm_gem_object *obj) WARN_ON(obj->dma_buf); if (obj->filp) - fput(obj->filp); + fput(obj->filp); } EXPORT_SYMBOL(drm_gem_object_release); @@ -782,7 +782,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; vma->vm_ops = dev->driver->gem_vm_ops; vma->vm_private_data = obj; - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); /* Take a ref for this mapping of the object, so that the fault * handler can dereference the mmap offset's pointer to the object. From 77472347972add74a3d89a0b9152b8eebc0ad2b0 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 20 Jan 2014 20:05:43 +0100 Subject: [PATCH 4/6] drm/gem: free vma-node during object-cleanup All drivers currently need to clean up the vma-node manually. There is no fancy logic involved so lets just clean it up unconditionally. The vma-manager correctly catches multiple calls so we are fine. Signed-off-by: David Herrmann --- drivers/gpu/drm/drm_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index dd8e38a22e23..5ea622c54e76 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -693,6 +693,8 @@ drm_gem_object_release(struct drm_gem_object *obj) if (obj->filp) fput(obj->filp); + + drm_gem_free_mmap_offset(obj); } EXPORT_SYMBOL(drm_gem_object_release); From b28cd41f9e9bb8085f7362c80833fc129628d3d6 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 20 Jan 2014 20:09:55 +0100 Subject: [PATCH 5/6] drm/crtc: add sanity checks to create_dumb() Lets make sure some basic expressions are always true: bpp != NULL width != NULL height != NULL stride = bpp * width < 2^32 size = stride * height < 2^32 PAGE_ALIGN(size) < 2^32 At least the udl driver doesn't check for multiplication-overflows, so lets just make sure it will never happen. These checks allow drivers to do any 32bit math without having to test for mult-overflows themselves. The two divisions might hurt performance a bit, but dumb_create() is only used for scanout-buffers, so that should be fine. We could use 64bit math to avoid the divisions, but that may be slow on 32bit machines.. Or maybe there should just be a "safe_mult32()" helper, which currently doesn't exist (I think?). Reviewed-by: Daniel Vetter Signed-off-by: David Herrmann --- drivers/gpu/drm/drm_crtc.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 35ea15d5ffff..b1c2b278005c 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3784,9 +3784,26 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_mode_create_dumb *args = data; + u32 cpp, stride, size; if (!dev->driver->dumb_create) return -ENOSYS; + if (!args->width || !args->height || !args->bpp) + return -EINVAL; + + /* overflow checks for 32bit size calculations */ + cpp = DIV_ROUND_UP(args->bpp, 8); + if (cpp > 0xffffffffU / args->width) + return -EINVAL; + stride = cpp * args->width; + if (args->height > 0xffffffffU / stride) + return -EINVAL; + + /* test for wrap-around */ + size = args->height * stride; + if (PAGE_ALIGN(size) == 0) + return -EINVAL; + return dev->driver->dumb_create(file_priv, dev, args); } From a8469aa81de532180846b22e8ead3d8f4d2f96a2 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 20 Jan 2014 20:15:38 +0100 Subject: [PATCH 6/6] drm/gem: dont init "ret" in drm_gem_mmap() There is no need to initialize this variable, so drop it. Otherwise, the compiler won't warn if we use it unintialized. Reviewed-by: Daniel Vetter Signed-off-by: David Herrmann --- drivers/gpu/drm/drm_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 5ea622c54e76..154d6c6955c1 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -820,7 +820,7 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) struct drm_device *dev = priv->minor->dev; struct drm_gem_object *obj; struct drm_vma_offset_node *node; - int ret = 0; + int ret; if (drm_device_is_unplugged(dev)) return -ENODEV;