[PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name

[PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_ioctl.c |   73 ++++++++++++++++++++++++++++++++----------
 1 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 9b9ff46..197267b 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -94,17 +94,24 @@ int drm_setunique(struct drm_device *dev, void *data,
 	master->unique_len = u->unique_len;
 	master->unique_size = u->unique_len + 1;
 	master->unique = kmalloc(master->unique_size, GFP_KERNEL);
-	if (!master->unique)
-		return -ENOMEM;
-	if (copy_from_user(master->unique, u->unique, master->unique_len))
-		return -EFAULT;
+	if (!master->unique) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	if (copy_from_user(master->unique, u->unique, master->unique_len)) {
+		ret = -EFAULT;
+		goto err;
+	}
 
 	master->unique[master->unique_len] = '\0';
 
 	dev->devname = kmalloc(strlen(dev->driver->pci_driver.name) +
 			       strlen(master->unique) + 2, GFP_KERNEL);
-	if (!dev->devname)
-		return -ENOMEM;
+	if (!dev->devname) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
 		master->unique);
@@ -113,24 +120,39 @@ int drm_setunique(struct drm_device *dev, void *data,
 	 * busid.
 	 */
 	ret = sscanf(master->unique, "PCI:%d:%d:%d", &bus, &slot, &func);
-	if (ret != 3)
-		return -EINVAL;
+	if (ret != 3) {
+		ret = -EINVAL;
+		goto err;
+	}
+
 	domain = bus >> 8;
 	bus &= 0xff;
 
 	if ((domain != drm_get_pci_domain(dev)) ||
 	    (bus != dev->pdev->bus->number) ||
 	    (slot != PCI_SLOT(dev->pdev->devfn)) ||
-	    (func != PCI_FUNC(dev->pdev->devfn)))
-		return -EINVAL;
+	    (func != PCI_FUNC(dev->pdev->devfn))) {
+		ret = -EINVAL;
+		goto err;
+	}
 
 	return 0;
+
+err:
+	kfree(dev->devname);
+	dev->devname = NULL;
+
+	kfree(master->unique);
+	master->unique = NULL;
+	master->unique_len = 0;
+	master->unique_size = 0;
+	return ret;
 }
 
 static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
 {
 	struct drm_master *master = file_priv->master;
-	int len;
+	int len, ret;
 
 	if (master->unique != NULL)
 		return -EBUSY;
@@ -138,28 +160,41 @@ static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
 	master->unique_len = 40;
 	master->unique_size = master->unique_len;
 	master->unique = kmalloc(master->unique_size, GFP_KERNEL);
-	if (master->unique == NULL)
-		return -ENOMEM;
+	if (master->unique == NULL) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	len = snprintf(master->unique, master->unique_len, "pci:%04x:%02x:%02x.%d",
 		       drm_get_pci_domain(dev),
 		       dev->pdev->bus->number,
 		       PCI_SLOT(dev->pdev->devfn),
 		       PCI_FUNC(dev->pdev->devfn));
-	if (len >= master->unique_len)
+	if (len >= master->unique_len) {
 		DRM_ERROR("buffer overflow");
-	else
+		ret = -EINVAL;
+		goto err;
+	} else
 		master->unique_len = len;
 
 	dev->devname = kmalloc(strlen(dev->driver->pci_driver.name) +
 			       master->unique_len + 2, GFP_KERNEL);
-	if (dev->devname == NULL)
-		return -ENOMEM;
+	if (dev->devname == NULL) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
 		master->unique);
 
 	return 0;
+
+err:
+	kfree(master->unique);
+	master->unique = NULL;
+	master->unique_len = 0;
+	master->unique_size = 0;
+	return ret;
 }
 
 /**
@@ -323,7 +358,9 @@ int drm_setversion(struct drm_device *dev, void *data, struct drm_file *file_pri
 			/*
 			 * Version 1.1 includes tying of DRM to specific device
 			 */
-			drm_set_busid(dev, file_priv);
+			retcode = drm_set_busid(dev, file_priv);
+			if (retcode)
+				goto done;
 		}
 	}
 
-- 
1.7.1

[PATCH 2/3] drm: Free devname along with master->unique

[Chris Wilson]

The device name is tightly coupled and created at the same time as the
master->unique address, so we need to free it with the master. Currently
we overwrite it each time we create a new master:

unreferenced object 0xe32c54b0 (size 32):
  comm "Xorg", pid 1455, jiffies 4294721798 (age 3196.879s)
  hex dump (first 32 bytes):
    69 39 31 35 40 70 63 69 3a 30 30 30 30 3a 30 30  i915@pci:0000:00
    3a 30 32 2e 30 00 6b 6b 6b 6b 6b 6b 6b 6b 6b a5  :02.0.kkkkkkkkk.
  backtrace:
    [<c04e5657>] create_object+0x124/0x1f1
    [<c07cf0f0>] kmemleak_alloc+0x4c/0x90
    [<c04db84c>] __kmalloc+0x155/0x175
    [<f8316665>] drm_setversion+0x11d/0x1b1 [drm]
    [<f83148d4>] drm_ioctl+0x29a/0x356 [drm]
    [<c04f27c4>] vfs_ioctl+0x33/0x91
    [<c04f31cf>] do_vfs_ioctl+0x46b/0x496
    [<c04f3240>] sys_ioctl+0x46/0x66
    [<c040325f>] sysenter_do_call+0x12/0x38
    [<ffffffff>] 0xffffffff

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_stub.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index a0c365f..4aa84f4 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -156,6 +156,9 @@ static void drm_master_destroy(struct kref *kref)
 		master->unique_len = 0;
 	}
 
+	kfree(dev->devname);
+	dev->devname = NULL;
+
 	list_for_each_entry_safe(pt, next, &master->magicfree, head) {
 		list_del(&pt->head);
 		drm_ht_remove_item(&master->magiclist, &pt->hash_item);
-- 
1.7.1

[PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure

[Chris Wilson]

A side-effect of being able to use custom page allocations with the
sg_table is that it cannot reap the partially constructed scatterlist if
fails to allocate a page. So we need to call sg_free_table() ourselves
if sg_alloc_table() fails.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc Dave Airlie <airlied@redhat.com>
---
 drivers/char/agp/intel-gtt.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index f97122a..5615d70 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -104,7 +104,7 @@ static int intel_agp_map_memory(struct agp_memory *mem)
 	DBG("try mapping %lu pages\n", (unsigned long)mem->page_count);
 
 	if (sg_alloc_table(&st, mem->page_count, GFP_KERNEL))
-		return -ENOMEM;
+		goto err;
 
 	mem->sg_list = sg = st.sgl;
 
@@ -113,11 +113,14 @@ static int intel_agp_map_memory(struct agp_memory *mem)
 
 	mem->num_sg = pci_map_sg(intel_private.pcidev, mem->sg_list,
 				 mem->page_count, PCI_DMA_BIDIRECTIONAL);
-	if (unlikely(!mem->num_sg)) {
-		intel_agp_free_sglist(mem);
-		return -ENOMEM;
-	}
+	if (unlikely(!mem->num_sg))
+		goto err;
+
 	return 0;
+
+err:
+	sg_free_table(&st);
+	return -ENOMEM;
 }
 
 static void intel_agp_unmap_memory(struct agp_memory *mem)
-- 
1.7.1

Re: [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name

[Chris Wilson]

On Sat, 24 Jul 2010 18:29:35 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> @@ -323,7 +358,9 @@ int drm_setversion(struct drm_device *dev, void *data, struct drm_file *file_pri
>  			/*
>  			 * Version 1.1 includes tying of DRM to specific device
>  			 */
> -			drm_set_busid(dev, file_priv);
> +			retcode = drm_set_busid(dev, file_priv);
> +			if (retcode)
> +				goto done;

And actually returning the error code is breaking xorg 1.8.99...
Hmm.

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm: Cleanup after failing to create master->unique and dev->name

[Chris Wilson]

v2: Userspace (notably xf86-video-{intel,ati}) became confused when
drmSetInterfaceVersion() started returning -EBUSY as they used a second
call (the first done in drmOpen()) to check their master credentials.
Since userspace wants to be able to repeatedly call
drmSetInterfaceVersion() allow them to do so.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_ioctl.c |   79 ++++++++++++++++++++++++++++++++----------
 1 files changed, 60 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 9b9ff46..f107fff 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -64,6 +64,19 @@ int drm_getunique(struct drm_device *dev, void *data,
 	return 0;
 }
 
+static void
+drm_unset_busid(struct drm_device *dev,
+		struct drm_master *master)
+{
+	kfree(dev->devname);
+	dev->devname = NULL;
+
+	kfree(master->unique);
+	master->unique = NULL;
+	master->unique_len = 0;
+	master->unique_size = 0;
+}
+
 /**
  * Set the bus id.
  *
@@ -94,17 +107,24 @@ int drm_setunique(struct drm_device *dev, void *data,
 	master->unique_len = u->unique_len;
 	master->unique_size = u->unique_len + 1;
 	master->unique = kmalloc(master->unique_size, GFP_KERNEL);
-	if (!master->unique)
-		return -ENOMEM;
-	if (copy_from_user(master->unique, u->unique, master->unique_len))
-		return -EFAULT;
+	if (!master->unique) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	if (copy_from_user(master->unique, u->unique, master->unique_len)) {
+		ret = -EFAULT;
+		goto err;
+	}
 
 	master->unique[master->unique_len] = '\0';
 
 	dev->devname = kmalloc(strlen(dev->driver->pci_driver.name) +
 			       strlen(master->unique) + 2, GFP_KERNEL);
-	if (!dev->devname)
-		return -ENOMEM;
+	if (!dev->devname) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
 		master->unique);
@@ -113,53 +133,72 @@ int drm_setunique(struct drm_device *dev, void *data,
 	 * busid.
 	 */
 	ret = sscanf(master->unique, "PCI:%d:%d:%d", &bus, &slot, &func);
-	if (ret != 3)
-		return -EINVAL;
+	if (ret != 3) {
+		ret = -EINVAL;
+		goto err;
+	}
+
 	domain = bus >> 8;
 	bus &= 0xff;
 
 	if ((domain != drm_get_pci_domain(dev)) ||
 	    (bus != dev->pdev->bus->number) ||
 	    (slot != PCI_SLOT(dev->pdev->devfn)) ||
-	    (func != PCI_FUNC(dev->pdev->devfn)))
-		return -EINVAL;
+	    (func != PCI_FUNC(dev->pdev->devfn))) {
+		ret = -EINVAL;
+		goto err;
+	}
 
 	return 0;
+
+err:
+	drm_unset_busid(dev, master);
+	return ret;
 }
 
 static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
 {
 	struct drm_master *master = file_priv->master;
-	int len;
+	int len, ret;
 
 	if (master->unique != NULL)
-		return -EBUSY;
+		drm_unset_busid(dev, master);
 
 	master->unique_len = 40;
 	master->unique_size = master->unique_len;
 	master->unique = kmalloc(master->unique_size, GFP_KERNEL);
-	if (master->unique == NULL)
-		return -ENOMEM;
+	if (master->unique == NULL) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	len = snprintf(master->unique, master->unique_len, "pci:%04x:%02x:%02x.%d",
 		       drm_get_pci_domain(dev),
 		       dev->pdev->bus->number,
 		       PCI_SLOT(dev->pdev->devfn),
 		       PCI_FUNC(dev->pdev->devfn));
-	if (len >= master->unique_len)
+	if (len >= master->unique_len) {
 		DRM_ERROR("buffer overflow");
-	else
+		ret = -EINVAL;
+		goto err;
+	} else
 		master->unique_len = len;
 
 	dev->devname = kmalloc(strlen(dev->driver->pci_driver.name) +
 			       master->unique_len + 2, GFP_KERNEL);
-	if (dev->devname == NULL)
-		return -ENOMEM;
+	if (dev->devname == NULL) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
 		master->unique);
 
 	return 0;
+
+err:
+	drm_unset_busid(dev, master);
+	return ret;
 }
 
 /**
@@ -323,7 +362,9 @@ int drm_setversion(struct drm_device *dev, void *data, struct drm_file *file_pri
 			/*
 			 * Version 1.1 includes tying of DRM to specific device
 			 */
-			drm_set_busid(dev, file_priv);
+			retcode = drm_set_busid(dev, file_priv);
+			if (retcode)
+				goto done;
 		}
 	}
 
-- 
1.7.1

Re: [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1893 bytes --]

On 2010.07.24 18:29:37 +0100, Chris Wilson wrote:
> A side-effect of being able to use custom page allocations with the
> sg_table is that it cannot reap the partially constructed scatterlist if
> fails to allocate a page. So we need to call sg_free_table() ourselves
> if sg_alloc_table() fails.

Why? Doesn't sg_alloc_table() handle the failure case to call sg_free_table()
already?

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc Dave Airlie <airlied@redhat.com>
> ---
>  drivers/char/agp/intel-gtt.c |   13 ++++++++-----
>  1 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index f97122a..5615d70 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -104,7 +104,7 @@ static int intel_agp_map_memory(struct agp_memory *mem)
>  	DBG("try mapping %lu pages\n", (unsigned long)mem->page_count);
>  
>  	if (sg_alloc_table(&st, mem->page_count, GFP_KERNEL))
> -		return -ENOMEM;
> +		goto err;
>  
>  	mem->sg_list = sg = st.sgl;
>  
> @@ -113,11 +113,14 @@ static int intel_agp_map_memory(struct agp_memory *mem)
>  
>  	mem->num_sg = pci_map_sg(intel_private.pcidev, mem->sg_list,
>  				 mem->page_count, PCI_DMA_BIDIRECTIONAL);
> -	if (unlikely(!mem->num_sg)) {
> -		intel_agp_free_sglist(mem);
> -		return -ENOMEM;
> -	}
> +	if (unlikely(!mem->num_sg))
> +		goto err;
> +
>  	return 0;
> +
> +err:
> +	sg_free_table(&st);
> +	return -ENOMEM;
>  }
>  
>  static void intel_agp_unmap_memory(struct agp_memory *mem)
> -- 
> 1.7.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure

[Chris Wilson]

On Tue, 27 Jul 2010 10:00:07 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> On 2010.07.24 18:29:37 +0100, Chris Wilson wrote:
> > A side-effect of being able to use custom page allocations with the
> > sg_table is that it cannot reap the partially constructed scatterlist if
> > fails to allocate a page. So we need to call sg_free_table() ourselves
> > if sg_alloc_table() fails.
> 
> Why? Doesn't sg_alloc_table() handle the failure case to call sg_free_table()
> already?

sg_alloc_table() is just a wrapper around __sg_alloc_table() that passes
in the default alloc_func. So for __sg_alloc_table() to do the cleanup we
would need to also pass in the free_func. Since it doesn't, it can't free
the partially constructed table. So to fix this we have the choice of
changing the interface (and add extra overhead to the common path) or
trivially rearrange our failure path.
-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm: Cleanup after failing to create master->unique and dev->name

[Dave Airlie]

On Sat, 2010-07-24 at 19:34 +0100, Chris Wilson wrote:
> v2: Userspace (notably xf86-video-{intel,ati}) became confused when
> drmSetInterfaceVersion() started returning -EBUSY as they used a second
> call (the first done in drmOpen()) to check their master credentials.
> Since userspace wants to be able to repeatedly call
> drmSetInterfaceVersion() allow them to do so.
> 

care to rebase on drm-core-next?

this fails to apply since the drm platform changes.

Dave.

Re: [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 349 bytes --]

On Sat, 24 Jul 2010 18:29:37 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> A side-effect of being able to use custom page allocations with the
> sg_table is that it cannot reap the partially constructed scatterlist if
> fails to allocate a page. So we need to call sg_free_table() ourselves
> if sg_alloc_table() fails.

Applied to -next.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: Cleanup after failing to create master->unique and dev->name

[Chris Wilson]

v2: Userspace (notably xf86-video-{intel,ati}) became confused when
drmSetInterfaceVersion() started returning -EBUSY as they used a second
call (the first done in drmOpen()) to check their master credentials.
Since userspace wants to be able to repeatedly call
drmSetInterfaceVersion() allow them to do so.

v3: Rebase to drm-core-next.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_ioctl.c |   85 +++++++++++++++++++++++++++++++++----------
 1 files changed, 66 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 76d3d18..7b03b19 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -64,6 +64,19 @@ int drm_getunique(struct drm_device *dev, void *data,
 	return 0;
 }
 
+static void
+drm_unset_busid(struct drm_device *dev,
+		struct drm_master *master)
+{
+	kfree(dev->devname);
+	dev->devname = NULL;
+
+	kfree(master->unique);
+	master->unique = NULL;
+	master->unique_len = 0;
+	master->unique_size = 0;
+}
+
 /**
  * Set the bus id.
  *
@@ -94,17 +107,24 @@ int drm_setunique(struct drm_device *dev, void *data,
 	master->unique_len = u->unique_len;
 	master->unique_size = u->unique_len + 1;
 	master->unique = kmalloc(master->unique_size, GFP_KERNEL);
-	if (!master->unique)
-		return -ENOMEM;
-	if (copy_from_user(master->unique, u->unique, master->unique_len))
-		return -EFAULT;
+	if (!master->unique) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	if (copy_from_user(master->unique, u->unique, master->unique_len)) {
+		ret = -EFAULT;
+		goto err;
+	}
 
 	master->unique[master->unique_len] = '\0';
 
 	dev->devname = kmalloc(strlen(dev->driver->pci_driver.name) +
 			       strlen(master->unique) + 2, GFP_KERNEL);
-	if (!dev->devname)
-		return -ENOMEM;
+	if (!dev->devname) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
 		master->unique);
@@ -113,24 +133,36 @@ int drm_setunique(struct drm_device *dev, void *data,
 	 * busid.
 	 */
 	ret = sscanf(master->unique, "PCI:%d:%d:%d", &bus, &slot, &func);
-	if (ret != 3)
-		return -EINVAL;
+	if (ret != 3) {
+		ret = -EINVAL;
+		goto err;
+	}
+
 	domain = bus >> 8;
 	bus &= 0xff;
 
 	if ((domain != drm_get_pci_domain(dev)) ||
 	    (bus != dev->pdev->bus->number) ||
 	    (slot != PCI_SLOT(dev->pdev->devfn)) ||
-	    (func != PCI_FUNC(dev->pdev->devfn)))
-		return -EINVAL;
+	    (func != PCI_FUNC(dev->pdev->devfn))) {
+		ret = -EINVAL;
+		goto err;
+	}
 
 	return 0;
+
+err:
+	drm_unset_busid(dev, master);
+	return ret;
 }
 
 static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
 {
 	struct drm_master *master = file_priv->master;
-	int len;
+	int len, ret;
+
+	if (master->unique != NULL)
+		drm_unset_busid(dev, master);
 
 	if (drm_core_check_feature(dev, DRIVER_USE_PLATFORM_DEVICE)) {
 		master->unique_len = 10 + strlen(dev->platformdev->name);
@@ -142,15 +174,20 @@ static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
 		len = snprintf(master->unique, master->unique_len,
 			"platform:%s", dev->platformdev->name);
 
-		if (len > master->unique_len)
+		if (len > master->unique_len) {
 			DRM_ERROR("Unique buffer overflowed\n");
+			ret = -EINVAL;
+			goto err;
+		}
 
 		dev->devname =
 			kmalloc(strlen(dev->platformdev->name) +
 				master->unique_len + 2, GFP_KERNEL);
 
-		if (dev->devname == NULL)
-			return -ENOMEM;
+		if (dev->devname == NULL) {
+			ret = -ENOMEM;
+			goto err;
+		}
 
 		sprintf(dev->devname, "%s@%s", dev->platformdev->name,
 			master->unique);
@@ -168,23 +205,31 @@ static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
 			dev->pdev->bus->number,
 			PCI_SLOT(dev->pdev->devfn),
 			PCI_FUNC(dev->pdev->devfn));
-		if (len >= master->unique_len)
+		if (len >= master->unique_len) {
 			DRM_ERROR("buffer overflow");
-		else
+			ret = -EINVAL;
+			goto err;
+		} else
 			master->unique_len = len;
 
 		dev->devname =
 			kmalloc(strlen(dev->driver->pci_driver.name) +
 				master->unique_len + 2, GFP_KERNEL);
 
-		if (dev->devname == NULL)
-			return -ENOMEM;
+		if (dev->devname == NULL) {
+			ret = -ENOMEM;
+			goto err;
+		}
 
 		sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
 			master->unique);
 	}
 
 	return 0;
+
+err:
+	drm_unset_busid(dev, master);
+	return ret;
 }
 
 /**
@@ -348,7 +393,9 @@ int drm_setversion(struct drm_device *dev, void *data, struct drm_file *file_pri
 			/*
 			 * Version 1.1 includes tying of DRM to specific device
 			 */
-			drm_set_busid(dev, file_priv);
+			retcode = drm_set_busid(dev, file_priv);
+			if (retcode)
+				goto done;
 		}
 	}
 
-- 
1.7.1