[PATCH v2 0/8] drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input

[Thierry Reding]

From: Thierry Reding <treding at nvidia.com>

This second version addresses review comments from before. A new patch
is added that removes a redundant call to drm_gem_free_mmap_offset() in
the CMA GEM helpers.

Below is the cover letter from the first version for more background in
case anybody missed it the last time:

Drivers currently treat the IOCTL data for DRM_IOCTL_MODE_CREATE_DUMB
inconsistently. While the documentation clearly states that the handle,
pitch and size fields are outputs, some drivers use their values as a
minimum hint from userspace, presumably to allow userspace to over-
allocate buffers on purpose (for example after negotiating pitch
alignment requirements with other devices).

Most of userspace is well-behaved in that in zeros out the output fields
which works well with drivers that use them as minima. However, some
userspace (like Mesa's GBM DRI backend) only initializes the inputs, so
that the driver will end up using uninitialized data from the stack in
the computations. This can cause the driver to attempt very large
allocations, resulting in failure, or silently use excessively  large
buffers.

This series tries to fix this by sanitizing the IOCTL data (setting the
output fields to 0 in the kernel) so that drivers can no longer use
uninitialized data. While at it, it also fixes existing drivers to not
treat output fields as input/output for consistency.

Note that this is technically breaking ABI since overallocating will no
longer be possible after this series is applied. However, the public DRM
headers have always documented the fields as being outputs, so any
application relying on them being inputs could be considered buggy. The
hope is that no such applications exist in the wild.

Discussion on IRC lead to the conclusion that new IOCTLs should have
input validation and require userspace to zero out output parameters to
avoid this kind of mess in the future. In order to help avoid this kind
of ambiguity it would be a good idea to start documenting IOCTLs more
officially (e.g. in the DRM DocBook).

I'm adding a bunch of people on Cc to get feedback about what userspace
people know might be relying on the current behaviour. Drivers that are
impacted are OMAP, R-Car and any that use GEM CMA helpers.

This series also contains a couple of preparatory patches that add the
GEM CMA helpers to our DocBook.

Thierry

Thierry Reding (8):
  drm/gem: Fix a few kerneldoc typos
  drm/doc: mm: Fix indentation
  drm/doc: Add GEM/CMA helpers to kerneldoc
  drm/cma: Introduce drm_gem_cma_dumb_create_internal()
  drm/omap: gem: dumb: pitch is an output
  drm/rcar: gem: dumb: pitch is an output
  drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input
  drm/cma: Remove call to drm_gem_free_mmap_offset()

 Documentation/DocBook/drm.tmpl        | 274 +++++++++++++++++-----------------
 drivers/gpu/drm/drm_crtc.c            |  10 ++
 drivers/gpu/drm/drm_gem.c             |  11 +-
 drivers/gpu/drm/drm_gem_cma_helper.c  | 259 ++++++++++++++++++++++++++------
 drivers/gpu/drm/omapdrm/omap_gem.c    |   2 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c |   4 +-
 include/drm/drm_gem_cma_helper.h      |  30 +++-
 7 files changed, 395 insertions(+), 195 deletions(-)

-- 
2.1.3