linux-kernel - [PATCH 3.2 17/27] drm: fix DRM_IOCTL_MODE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 29 Dec 2014 02:11:31 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:	akpm@...ux-foundation.org,
	"Chris Wilson" <chris@...is-wilson.co.uk>,
	"David Herrmann" <dh.herrmann@...il.com>,
	"Dave Airlie" <airlied@...hat.com>
Subject: [PATCH 3.2 17/27] drm: fix DRM_IOCTL_MODE_GETFB handle-leak

3.2.66-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: David Herrmann <dh.herrmann@...il.com>

commit 101b96f32956ee99bf1468afaf572b88cda9f88b 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 a14b1b42477c5ef089fcda88cbaae50d979eb8f9
  Author: Mandeep Singh Baines <mandeep.baines@...il.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: David Herrmann <dh.herrmann@...il.com>
Reviewed-by: Chris Wilson <chris@...is-wilson.co.uk>
Signed-off-by: Dave Airlie <airlied@...hat.com>
[bwh: Backported to 3.2:
 - drm_framebuffer_funcs::create_handle must be non-null
 - Adjust context, indentation]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1815,7 +1815,17 @@ int drm_mode_getfb(struct drm_device *de
 	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);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/