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]
Message-ID: <lsq.1450971462.1055459@decadent.org.uk>
Date:	Thu, 24 Dec 2015 15:37:42 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:	akpm@...ux-foundation.org,
	"Thomas Hellstrom" <thellstrom@...are.com>,
	"Dave Airlie" <airlied@...hat.com>
Subject: [PATCH 3.2 46/77] drm: Fix an unwanted master inheritance v2

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

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

From: Thomas Hellstrom <thellstrom@...are.com>

commit a0af2e538c80f3e47f1d6ddf120a153ad909e8ad upstream.

A client calling drmSetMaster() using a file descriptor that was opened
when another client was master would inherit the latter client's master
object and all its authenticated clients.

This is unwanted behaviour, and when this happens, instead allocate a
brand new master object for the client calling drmSetMaster().

Fixes a BUG() throw in vmw_master_set().

Signed-off-by: Thomas Hellstrom <thellstrom@...are.com>
Signed-off-by: Dave Airlie <airlied@...hat.com>
[bwh: Backported to 3.2:
 - s/master_mutex/struct_mutex/
 - drm_new_set_master() must drop struct_mutex while calling
   drm_driver::master_create
 - Adjust filename, context, indentation]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -225,6 +225,10 @@ int drm_setmaster_ioctl(struct drm_devic
 	if (!file_priv->minor->master &&
 	    file_priv->minor->master != file_priv->master) {
 		mutex_lock(&dev->struct_mutex);
+		if (!file_priv->allowed_master) {
+			ret = drm_new_set_master(dev, file_priv);
+			goto out_unlock;
+		}
 		file_priv->minor->master = drm_master_get(file_priv->master);
 		file_priv->is_master = 1;
 		if (dev->driver->master_set) {
@@ -234,10 +238,11 @@ int drm_setmaster_ioctl(struct drm_devic
 				drm_master_put(&file_priv->minor->master);
 			}
 		}
+	out_unlock:
 		mutex_unlock(&dev->struct_mutex);
 	}
 
-	return 0;
+	return ret;
 }
 
 int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -219,6 +219,62 @@ static int drm_cpu_valid(void)
 }
 
 /**
+ * drm_new_set_master - Allocate a new master object and become master for the
+ * associated master realm.
+ *
+ * @dev: The associated device.
+ * @fpriv: File private identifying the client.
+ *
+ * This function must be called with dev::struct_mutex held.
+ * Returns negative error code on failure. Zero on success.
+ */
+int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
+{
+	struct drm_master *old_master;
+	int ret;
+
+	lockdep_assert_held_once(&dev->struct_mutex);
+
+	/* create a new master */
+	fpriv->minor->master = drm_master_create(fpriv->minor);
+	if (!fpriv->minor->master)
+		return -ENOMEM;
+
+	/* take another reference for the copy in the local file priv */
+	old_master = fpriv->master;
+	fpriv->master = drm_master_get(fpriv->minor->master);
+
+	if (dev->driver->master_create) {
+		mutex_unlock(&dev->struct_mutex);
+		ret = dev->driver->master_create(dev, fpriv->master);
+		mutex_lock(&dev->struct_mutex);
+		if (ret)
+			goto out_err;
+	}
+	if (dev->driver->master_set) {
+		ret = dev->driver->master_set(dev, fpriv, true);
+		if (ret)
+			goto out_err;
+	}
+
+	fpriv->is_master = 1;
+	fpriv->allowed_master = 1;
+	fpriv->authenticated = 1;
+	if (old_master)
+		drm_master_put(&old_master);
+
+	return 0;
+
+out_err:
+	/* drop both references and restore old master on failure */
+	drm_master_put(&fpriv->minor->master);
+	drm_master_put(&fpriv->master);
+	fpriv->master = old_master;
+
+	return ret;
+}
+
+/**
  * Called whenever a process opens /dev/drm.
  *
  * \param inode device inode.
@@ -279,43 +335,10 @@ static int drm_open_helper(struct inode
 	mutex_lock(&dev->struct_mutex);
 	if (!priv->minor->master) {
 		/* create a new master */
-		priv->minor->master = drm_master_create(priv->minor);
-		if (!priv->minor->master) {
-			mutex_unlock(&dev->struct_mutex);
-			ret = -ENOMEM;
-			goto out_free;
-		}
-
-		priv->is_master = 1;
-		/* take another reference for the copy in the local file priv */
-		priv->master = drm_master_get(priv->minor->master);
-
-		priv->authenticated = 1;
-
-		mutex_unlock(&dev->struct_mutex);
-		if (dev->driver->master_create) {
-			ret = dev->driver->master_create(dev, priv->master);
-			if (ret) {
-				mutex_lock(&dev->struct_mutex);
-				/* drop both references if this fails */
-				drm_master_put(&priv->minor->master);
-				drm_master_put(&priv->master);
-				mutex_unlock(&dev->struct_mutex);
-				goto out_free;
-			}
-		}
-		mutex_lock(&dev->struct_mutex);
-		if (dev->driver->master_set) {
-			ret = dev->driver->master_set(dev, priv, true);
-			if (ret) {
-				/* drop both references if this fails */
-				drm_master_put(&priv->minor->master);
-				drm_master_put(&priv->master);
-				mutex_unlock(&dev->struct_mutex);
-				goto out_free;
-			}
-		}
+		ret = drm_new_set_master(dev, priv);
 		mutex_unlock(&dev->struct_mutex);
+		if (ret)
+			goto out_free;
 	} else {
 		/* get a reference to the master */
 		priv->master = drm_master_get(priv->minor->master);
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -430,6 +430,11 @@ struct drm_file {
 	void *driver_priv;
 
 	int is_master; /* this file private is a master for a minor */
+	/*
+	 * This client is allowed to gain master privileges for @master.
+	 * Protected by struct drm_device::struct_mutex.
+	 */
+	unsigned allowed_master:1;
 	struct drm_master *master; /* master this node is currently associated with
 				      N.B. not always minor->master */
 	struct list_head fbs;
@@ -1254,6 +1259,7 @@ extern int drm_fasync(int fd, struct fil
 extern ssize_t drm_read(struct file *filp, char __user *buffer,
 			size_t count, loff_t *offset);
 extern int drm_release(struct inode *inode, struct file *filp);
+extern int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv);
 
 				/* Mapping support (drm_vm.h) */
 extern int drm_mmap(struct file *filp, struct vm_area_struct *vma);

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ