Doug / smarc-fsl-linux-kernel

Download as
Browse Code ยป

Commit 101b96f32956ee99bf1468afaf572b88cda9f88b

Authored by David Herrmann
Committed by Dave Airlie
1 parent a3376e3ec8
Exists in smarc-imx_3.14.28_1.0.0_ga and in 1 other branch imx_3.14.28_1.0.0_ga

drm: fix DRM_IOCTL_MODE_GETFB handle-leak 

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 a14b1b42477c5ef089fcda88cbaae50d979eb8f9
  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

Cc: <stable@vger.kernel.org>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dave Airlie <airlied@redhat.com>

Showing 1 changed file with 15 additions and 3 deletions Side-by-side Diff

drivers/gpu/drm/drm_crtc.c
Diff comments   View file @ 101b96f
... ... @@ -2641,10 +2641,22 @@
2641 2641 r->depth = fb->depth;
2642 2642 r->bpp = fb->bits_per_pixel;
2643 2643 r->pitch = fb->pitches[0];
2644   - if (fb->funcs->create_handle)
2645   - ret = fb->funcs->create_handle(fb, file_priv, &r->handle);
2646   - else
  2644 + if (fb->funcs->create_handle) {
  2645 + if (file_priv->is_master || capable(CAP_SYS_ADMIN)) {
  2646 + ret = fb->funcs->create_handle(fb, file_priv,
  2647 + &r->handle);
  2648 + } else {
  2649 + /* GET_FB() is an unprivileged ioctl so we must not
  2650 + * return a buffer-handle to non-master processes! For
  2651 + * backwards-compatibility reasons, we cannot make
  2652 + * GET_FB() privileged, so just return an invalid handle
  2653 + * for non-masters. */
  2654 + r->handle = 0;
  2655 + ret = 0;
  2656 + }
  2657 + } else {
2647 2658 ret = -ENODEV;
  2659 + }
2648 2660  
2649 2661 drm_framebuffer_unreference(fb);
2650 2662