[PATCH] drm/i915: Reorder phys backing storage release

* [PATCH] drm/i915: Reorder phys backing storage release
@ 2016-12-07 10:07 Chris Wilson
  2016-12-07 11:45 ` ✓ Fi.CI.BAT: success for " Patchwork
  2016-12-07 12:10 ` [PATCH] " Joonas Lahtinen
  0 siblings, 2 replies; 5+ messages in thread
From: Chris Wilson @ 2016-12-07 10:07 UTC (permalink / raw)
  To: intel-gfx

In commit a4f5ea64f0a8 ("drm/i915: Refactor object page API"), I
reordered the object->pages teardown to be more friendly wrt to a
separate obj->mm.lock. However, I overlooked the phys object and left it
with a dangling use-after-free of its phys_handle. Move the allocation
of the phys handle to get_pages and it release to put_pages to prevent
the invalid access and to improve symmetry.

Testcase: igt/drv_selftest/objects
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fixes: a4f5ea64f0a8 ("drm/i915: Refactor object page API")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/i915/i915_gem.c | 43 +++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ca0bb837a57f..f05c48dce7d0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -176,21 +176,29 @@ static struct sg_table *
 i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 {
 	struct address_space *mapping = obj->base.filp->f_mapping;
-	char *vaddr = obj->phys_handle->vaddr;
+	drm_dma_handle_t *phys;
 	struct sg_table *st;
 	struct scatterlist *sg;
+	char *vaddr;
 	int i;
 
 	if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
 		return ERR_PTR(-EINVAL);
 
+	phys = drm_pci_alloc(obj->base.dev, obj->base.size, obj->base.size);
+	if (!phys)
+		return ERR_PTR(-ENOMEM);
+
+	vaddr = phys->vaddr;
 	for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
 		struct page *page;
 		char *src;
 
 		page = shmem_read_mapping_page(mapping, i);
-		if (IS_ERR(page))
-			return ERR_CAST(page);
+		if (IS_ERR(page)) {
+			st = ERR_CAST(page);
+			goto err_phys;
+		}
 
 		src = kmap_atomic(page);
 		memcpy(vaddr, src, PAGE_SIZE);
@@ -204,21 +212,29 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 	i915_gem_chipset_flush(to_i915(obj->base.dev));
 
 	st = kmalloc(sizeof(*st), GFP_KERNEL);
-	if (st == NULL)
-		return ERR_PTR(-ENOMEM);
+	if (st == NULL) {
+		st = ERR_PTR(-ENOMEM);
+		goto err_phys;
+	}
 
 	if (sg_alloc_table(st, 1, GFP_KERNEL)) {
 		kfree(st);
-		return ERR_PTR(-ENOMEM);
+		st = ERR_PTR(-ENOMEM);
+		goto err_phys;
 	}
 
 	sg = st->sgl;
 	sg->offset = 0;
 	sg->length = obj->base.size;
 
-	sg_dma_address(sg) = obj->phys_handle->busaddr;
+	sg_dma_address(sg) = phys->busaddr;
 	sg_dma_len(sg) = obj->base.size;
 
+	obj->phys_handle = phys;
+	return st;
+
+err_phys:
+	drm_pci_free(obj->base.dev, phys);
 	return st;
 }
 
@@ -274,12 +290,13 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj,
 
 	sg_free_table(pages);
 	kfree(pages);
+
+	drm_pci_free(obj->base.dev, obj->phys_handle);
 }
 
 static void
 i915_gem_object_release_phys(struct drm_i915_gem_object *obj)
 {
-	drm_pci_free(obj->base.dev, obj->phys_handle);
 	i915_gem_object_unpin_pages(obj);
 }
 
@@ -540,9 +557,11 @@ int
 i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
 			    int align)
 {
-	drm_dma_handle_t *phys;
 	int ret;
 
+	if (align > obj->base.size)
+		return -EINVAL;
+
 	if (obj->phys_handle) {
 		if ((unsigned long)obj->phys_handle->vaddr & (align -1))
 			return -EBUSY;
@@ -564,12 +583,6 @@ i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
 	if (obj->mm.pages)
 		return -EBUSY;
 
-	/* create a new object */
-	phys = drm_pci_alloc(obj->base.dev, obj->base.size, align);
-	if (!phys)
-		return -ENOMEM;
-
-	obj->phys_handle = phys;
 	obj->ops = &i915_gem_phys_ops;
 
 	return i915_gem_object_pin_pages(obj);
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread