Merge tag 'drm/gem-cma/for-3.19-rc1' of git://people.freedesktop.org/~tagr/linux into drm-next

drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input

Some drivers erroneously treat the .pitch and .size fields of struct
drm_mode_create_dumb as inputs. While the include/uapi/drm/drm_mode.h
header has a comment denoting them as outputs, that seemingly wasn't
enough to make drivers use them properly.

The result is that some userspace doesn't explicitly zero out those
fields, assuming that the kernel won't use them. That causes problems
since the data within the structure might be uninitialized, so bogus
data may end up confusing drivers (ridiculously large values for the
pitch, ...).

This series attempts to improve the situation by fixing all drivers to
not use the output fields. Furthermore to spare new drivers this bad
surprise, the DRM core now zeros out these fields prior to handing the
data structure to the driver.

Lessons learned from this are that future IOCTLs should be properly
documented (in the DRM DocBook for example) and should be rigorously
defined. To prevent misuse like this, userspace should be required to
zero out all output fields. The kernel should check for this and fail
if that's not the case.

* tag 'drm/gem-cma/for-3.19-rc1' of git://people.freedesktop.org/~tagr/linux:
  drm/cma: Remove call to drm_gem_free_mmap_offset()
  drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input
  drm/rcar: gem: dumb: pitch is an output
  drm/omap: gem: dumb: pitch is an output
  drm/cma: Introduce drm_gem_cma_dumb_create_internal()
  drm/doc: Add GEM/CMA helpers to kerneldoc
  drm/doc: mm: Fix indentation
  drm/gem: Fix a few kerneldoc typos

drivers/gpu/drm/drm_crtc.c

@@ -4774,6 +4774,16 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
 	if (PAGE_ALIGN(size) == 0)
 		return -EINVAL;
 
+	/*
+	 * handle, pitch and size are output parameters. Zero them out to
+	 * prevent drivers from accidentally using uninitialized data. Since
+	 * not all existing userspace is clearing these fields properly we
+	 * cannot reject IOCTL with garbage in them.
+	 */
+	args->handle = 0;
+	args->pitch = 0;
+	args->size = 0;
+
 	return dev->driver->dumb_create(file_priv, dev, args);
 }
 

drivers/gpu/drm/drm_gem.c

@@ -309,7 +309,7 @@ EXPORT_SYMBOL(drm_gem_dumb_destroy);
  * drm_gem_handle_create_tail - internal functions to create a handle
  * @file_priv: drm file-private structure to register the handle for
  * @obj: object to register
- * @handlep: pionter to return the created handle to the caller
+ * @handlep: pointer to return the created handle to the caller
  * 
  * This expects the dev->object_name_lock to be held already and will drop it
  * before returning. Used to avoid races in establishing new handles when
@@ -362,7 +362,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 }
 
 /**
- * gem_handle_create - create a gem handle for an object
+ * drm_gem_handle_create - create a gem handle for an object
  * @file_priv: drm file-private structure to register the handle for
  * @obj: object to register
  * @handlep: pionter to return the created handle to the caller
@@ -371,10 +371,9 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
  * to the object, which includes a regular reference count. Callers
  * will likely want to dereference the object afterwards.
  */
-int
-drm_gem_handle_create(struct drm_file *file_priv,
-		       struct drm_gem_object *obj,
-		       u32 *handlep)
+int drm_gem_handle_create(struct drm_file *file_priv,
+			  struct drm_gem_object *obj,
+			  u32 *handlep)
 {
 	mutex_lock(&obj->dev->object_name_lock);
 

drivers/gpu/drm/omapdrm/omap_gem.c

@@ -612,8 +612,7 @@ int omap_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
 {
 	union omap_gem_size gsize;
 
-	/* in case someone tries to feed us a completely bogus stride: */
-	args->pitch = align_pitch(args->pitch, args->width, args->bpp);
+	args->pitch = align_pitch(0, args->width, args->bpp);
 	args->size = PAGE_ALIGN(args->pitch * args->height);
 
 	gsize = (union omap_gem_size){

drivers/gpu/drm/rcar-du/rcar_du_kms.c

@@ -126,9 +126,9 @@ int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
 	else
 		align = 16 * args->bpp / 8;
 
-	args->pitch = roundup(max(args->pitch, min_pitch), align);
+	args->pitch = roundup(min_pitch, align);
 
-	return drm_gem_cma_dumb_create(file, dev, args);
+	return drm_gem_cma_dumb_create_internal(file, dev, args);
 }
 
 static struct drm_framebuffer *

include/drm/drm_gem_cma_helper.h

@@ -4,6 +4,13 @@
 #include <drm/drmP.h>
 #include <drm/drm_gem.h>
 
+/**
+ * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
+ * @base: base GEM object
+ * @paddr: physical address of the backing memory
+ * @sgt: scatter/gather table for imported PRIME buffers
+ * @vaddr: kernel virtual address of the backing memory
+ */
 struct drm_gem_cma_object {
 	struct drm_gem_object base;
 	dma_addr_t paddr;
@@ -19,23 +26,30 @@ to_drm_gem_cma_obj(struct drm_gem_object *gem_obj)
 	return container_of(gem_obj, struct drm_gem_cma_object, base);
 }
 
-/* free gem object. */
+/* free GEM object */
 void drm_gem_cma_free_object(struct drm_gem_object *gem_obj);
 
-/* create memory region for drm framebuffer. */
+/* create memory region for DRM framebuffer */
+int drm_gem_cma_dumb_create_internal(struct drm_file *file_priv,
+				     struct drm_device *drm,
+				     struct drm_mode_create_dumb *args);
+
+/* create memory region for DRM framebuffer */
 int drm_gem_cma_dumb_create(struct drm_file *file_priv,
-		struct drm_device *drm, struct drm_mode_create_dumb *args);
+			    struct drm_device *drm,
+			    struct drm_mode_create_dumb *args);
 
-/* map memory region for drm framebuffer to user space. */
+/* map memory region for DRM framebuffer to user space */
 int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,
-		struct drm_device *drm, uint32_t handle, uint64_t *offset);
+				struct drm_device *drm, u32 handle,
+				u64 *offset);
 
-/* set vm_flags and we can change the vm attribute to other one at here. */
+/* set vm_flags and we can change the VM attribute to other one at here */
 int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma);
 
-/* allocate physical memory. */
+/* allocate physical memory */
 struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
-		unsigned int size);
+					      size_t size);
 
 extern const struct vm_operations_struct drm_gem_cma_vm_ops;