Commit 125f21f3 authored by David Herrmann's avatar David Herrmann Committed by Ben Hutchings

drm: fix DRM_IOCTL_MODE_GETFB handle-leak

commit 101b96f3 upstream.

DRM_IOCTL_MODE_GETFB is used to retrieve information about a given
framebuffer ID. It is a read-only helper and was thus declassified for
unprivileged access in:

  commit a14b1b42
  Author: Mandeep Singh Baines <mandeep.baines@gmail.com>
  Date:   Fri Jan 20 12:11:16 2012 -0800

      drm: remove master fd restriction on mode setting getters

However, alongside width, height and stride information,
DRM_IOCTL_MODE_GETFB also passes back a handle to the underlying buffer of
the framebuffer. This handle allows users to mmap() it and read or write
into it. Obviously, this should be restricted to DRM-Master.

With the current setup, *any* process with access to /dev/dri/card0 (which
means any process with access to hardware-accelerated rendering) can
access the current screen framebuffer and modify it ad libitum.

For backwards-compatibility reasons we want to keep the
DRM_IOCTL_MODE_GETFB call unprivileged. Besides, it provides quite useful
information regarding screen setup. So we simply test whether the caller
is the current DRM-Master and if not, we return 0 as handle, which is
always invalid. A following DRM_IOCTL_GEM_CLOSE on this handle will fail
with EINVAL, but we accept this. Users shouldn't test for errors during
GEM_CLOSE, anyway. And it is still better as a failing MODE_GETFB call.

v2: add capable(CAP_SYS_ADMIN) check for compatibility with i-g-t
Signed-off-by: default avatarDavid Herrmann <dh.herrmann@gmail.com>
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarDave Airlie <airlied@redhat.com>
[bwh: Backported to 3.2:
 - drm_framebuffer_funcs::create_handle must be non-null
 - Adjust context, indentation]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent 17d1b500
......@@ -1815,7 +1815,17 @@ int drm_mode_getfb(struct drm_device *dev,
r->depth = fb->depth;
r->bpp = fb->bits_per_pixel;
r->pitch = fb->pitch;
fb->funcs->create_handle(fb, file_priv, &r->handle);
if (file_priv->is_master || capable(CAP_SYS_ADMIN)) {
ret = fb->funcs->create_handle(fb, file_priv, &r->handle);
} else {
/* GET_FB() is an unprivileged ioctl so we must not
* return a buffer-handle to non-master processes! For
* backwards-compatibility reasons, we cannot make
* GET_FB() privileged, so just return an invalid handle
* for non-masters. */
r->handle = 0;
ret = 0;
}
out:
mutex_unlock(&dev->mode_config.mutex);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment