* [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name
@ 2010-07-24 17:29 Chris Wilson
2010-07-24 17:29 ` [PATCH 2/3] drm: Free devname along with master->unique Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Chris Wilson @ 2010-07-24 17:29 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/3] drm: Free devname along with master->unique
2010-07-24 17:29 [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name Chris Wilson
@ 2010-07-24 17:29 ` Chris Wilson
2010-07-24 17:29 ` [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure Chris Wilson
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2010-07-24 17:29 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure
2010-07-24 17:29 [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name Chris Wilson
2010-07-24 17:29 ` [PATCH 2/3] drm: Free devname along with master->unique Chris Wilson
@ 2010-07-24 17:29 ` Chris Wilson
2010-07-27 2:00 ` Zhenyu Wang
2010-08-02 2:26 ` Eric Anholt
2010-07-24 17:56 ` [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name Chris Wilson
2010-07-24 18:34 ` [PATCH] " Chris Wilson
3 siblings, 2 replies; 10+ messages in thread
From: Chris Wilson @ 2010-07-24 17:29 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure
2010-07-24 17:29 ` [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure Chris Wilson
@ 2010-07-27 2:00 ` Zhenyu Wang
2010-07-27 7:42 ` Chris Wilson
2010-08-02 2:26 ` Eric Anholt
1 sibling, 1 reply; 10+ messages in thread
From: Zhenyu Wang @ 2010-07-27 2:00 UTC (permalink / raw)
To: Chris Wilson; +Cc: dri-devel
[-- 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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure
2010-07-27 2:00 ` Zhenyu Wang
@ 2010-07-27 7:42 ` Chris Wilson
0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2010-07-27 7:42 UTC (permalink / raw)
To: Zhenyu Wang; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure
2010-07-24 17:29 ` [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure Chris Wilson
2010-07-27 2:00 ` Zhenyu Wang
@ 2010-08-02 2:26 ` Eric Anholt
1 sibling, 0 replies; 10+ messages in thread
From: Eric Anholt @ 2010-08-02 2:26 UTC (permalink / raw)
To: Chris Wilson, dri-devel
[-- 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name
2010-07-24 17:29 [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name Chris Wilson
2010-07-24 17:29 ` [PATCH 2/3] drm: Free devname along with master->unique Chris Wilson
2010-07-24 17:29 ` [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure Chris Wilson
@ 2010-07-24 17:56 ` Chris Wilson
2010-07-24 18:34 ` [PATCH] " Chris Wilson
3 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2010-07-24 17:56 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] drm: Cleanup after failing to create master->unique and dev->name
2010-07-24 17:29 [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name Chris Wilson
` (2 preceding siblings ...)
2010-07-24 17:56 ` [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name Chris Wilson
@ 2010-07-24 18:34 ` Chris Wilson
2010-08-02 0:13 ` Dave Airlie
3 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2010-07-24 18:34 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] drm: Cleanup after failing to create master->unique and dev->name
2010-07-24 18:34 ` [PATCH] " Chris Wilson
@ 2010-08-02 0:13 ` Dave Airlie
0 siblings, 0 replies; 10+ messages in thread
From: Dave Airlie @ 2010-08-02 0:13 UTC (permalink / raw)
To: Chris Wilson; +Cc: dri-devel
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] drm: Cleanup after failing to create master->unique and dev->name
@ 2010-08-04 10:09 Chris Wilson
0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2010-08-04 10:09 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-08-04 10:10 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-24 17:29 [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name Chris Wilson
2010-07-24 17:29 ` [PATCH 2/3] drm: Free devname along with master->unique Chris Wilson
2010-07-24 17:29 ` [PATCH 3/3] agp/intel: Destroy the scatterlist on allocation failure Chris Wilson
2010-07-27 2:00 ` Zhenyu Wang
2010-07-27 7:42 ` Chris Wilson
2010-08-02 2:26 ` Eric Anholt
2010-07-24 17:56 ` [PATCH 1/3] drm: Cleanup after failing to create master->unique and dev->name Chris Wilson
2010-07-24 18:34 ` [PATCH] " Chris Wilson
2010-08-02 0:13 ` Dave Airlie
-- strict thread matches above, loose matches on Subject: below --
2010-08-04 10:09 Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.