Commit 234896a7 authored by Luca Barbieri's avatar Luca Barbieri Committed by Ben Skeggs

drm/nouveau: Fix null deref in nouveau_fence_emit due to deleted fence

Currently Nouveau will unvalidate all buffers if it is forced to wait on
one, and then start revalidating from the beginning.  While doing so, it
destroys the operation fence, causing nouveau_fence_emit to crash.

This patch fixes this bug by taking the fence object out of validate_op
and creating it just before emit.  The fence pointer is initialized to 0
and unref'ed unconditionally.

In addition to fixing the bug, this prevents its reintroduction and
simplifies the code.
Signed-off-by: default avatarLuca Barbieri <luca@luca-barbieri.com>
Signed-off-by: default avatarBen Skeggs <bskeggs@redhat.com>
parent 7978b9cf
...@@ -220,7 +220,6 @@ nouveau_gem_set_domain(struct drm_gem_object *gem, uint32_t read_domains, ...@@ -220,7 +220,6 @@ nouveau_gem_set_domain(struct drm_gem_object *gem, uint32_t read_domains,
} }
struct validate_op { struct validate_op {
struct nouveau_fence *fence;
struct list_head vram_list; struct list_head vram_list;
struct list_head gart_list; struct list_head gart_list;
struct list_head both_list; struct list_head both_list;
...@@ -252,17 +251,11 @@ validate_fini_list(struct list_head *list, struct nouveau_fence *fence) ...@@ -252,17 +251,11 @@ validate_fini_list(struct list_head *list, struct nouveau_fence *fence)
} }
static void static void
validate_fini(struct validate_op *op, bool success) validate_fini(struct validate_op *op, struct nouveau_fence* fence)
{ {
struct nouveau_fence *fence = op->fence; validate_fini_list(&op->vram_list, fence);
validate_fini_list(&op->gart_list, fence);
if (unlikely(!success)) validate_fini_list(&op->both_list, fence);
op->fence = NULL;
validate_fini_list(&op->vram_list, op->fence);
validate_fini_list(&op->gart_list, op->fence);
validate_fini_list(&op->both_list, op->fence);
nouveau_fence_unref((void *)&fence);
} }
static int static int
...@@ -420,10 +413,6 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan, ...@@ -420,10 +413,6 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
INIT_LIST_HEAD(&op->gart_list); INIT_LIST_HEAD(&op->gart_list);
INIT_LIST_HEAD(&op->both_list); INIT_LIST_HEAD(&op->both_list);
ret = nouveau_fence_new(chan, &op->fence, false);
if (ret)
return ret;
if (nr_buffers == 0) if (nr_buffers == 0)
return 0; return 0;
...@@ -541,6 +530,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, ...@@ -541,6 +530,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
struct drm_nouveau_gem_pushbuf_bo *bo = NULL; struct drm_nouveau_gem_pushbuf_bo *bo = NULL;
struct nouveau_channel *chan; struct nouveau_channel *chan;
struct validate_op op; struct validate_op op;
struct nouveau_fence* fence = 0;
uint32_t *pushbuf = NULL; uint32_t *pushbuf = NULL;
int ret = 0, do_reloc = 0, i; int ret = 0, do_reloc = 0, i;
...@@ -597,7 +587,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, ...@@ -597,7 +587,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
OUT_RINGp(chan, pushbuf, req->nr_dwords); OUT_RINGp(chan, pushbuf, req->nr_dwords);
ret = nouveau_fence_emit(op.fence); ret = nouveau_fence_new(chan, &fence, true);
if (ret) { if (ret) {
NV_ERROR(dev, "error fencing pushbuf: %d\n", ret); NV_ERROR(dev, "error fencing pushbuf: %d\n", ret);
WIND_RING(chan); WIND_RING(chan);
...@@ -605,7 +595,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, ...@@ -605,7 +595,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
} }
if (nouveau_gem_pushbuf_sync(chan)) { if (nouveau_gem_pushbuf_sync(chan)) {
ret = nouveau_fence_wait(op.fence, NULL, false, false); ret = nouveau_fence_wait(fence, NULL, false, false);
if (ret) { if (ret) {
for (i = 0; i < req->nr_dwords; i++) for (i = 0; i < req->nr_dwords; i++)
NV_ERROR(dev, "0x%08x\n", pushbuf[i]); NV_ERROR(dev, "0x%08x\n", pushbuf[i]);
...@@ -614,7 +604,8 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, ...@@ -614,7 +604,8 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
} }
out: out:
validate_fini(&op, ret == 0); validate_fini(&op, fence);
nouveau_fence_unref((void**)&fence);
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
kfree(pushbuf); kfree(pushbuf);
kfree(bo); kfree(bo);
...@@ -634,6 +625,7 @@ nouveau_gem_ioctl_pushbuf_call(struct drm_device *dev, void *data, ...@@ -634,6 +625,7 @@ nouveau_gem_ioctl_pushbuf_call(struct drm_device *dev, void *data,
struct drm_gem_object *gem; struct drm_gem_object *gem;
struct nouveau_bo *pbbo; struct nouveau_bo *pbbo;
struct validate_op op; struct validate_op op;
struct nouveau_fence* fence = 0;
int i, ret = 0, do_reloc = 0; int i, ret = 0, do_reloc = 0;
NOUVEAU_CHECK_INITIALISED_WITH_RETURN; NOUVEAU_CHECK_INITIALISED_WITH_RETURN;
...@@ -772,7 +764,7 @@ nouveau_gem_ioctl_pushbuf_call(struct drm_device *dev, void *data, ...@@ -772,7 +764,7 @@ nouveau_gem_ioctl_pushbuf_call(struct drm_device *dev, void *data,
OUT_RING(chan, 0); OUT_RING(chan, 0);
} }
ret = nouveau_fence_emit(op.fence); ret = nouveau_fence_new(chan, &fence, true);
if (ret) { if (ret) {
NV_ERROR(dev, "error fencing pushbuf: %d\n", ret); NV_ERROR(dev, "error fencing pushbuf: %d\n", ret);
WIND_RING(chan); WIND_RING(chan);
...@@ -780,7 +772,8 @@ nouveau_gem_ioctl_pushbuf_call(struct drm_device *dev, void *data, ...@@ -780,7 +772,8 @@ nouveau_gem_ioctl_pushbuf_call(struct drm_device *dev, void *data,
} }
out: out:
validate_fini(&op, ret == 0); validate_fini(&op, fence);
nouveau_fence_unref((void**)&fence);
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
kfree(bo); kfree(bo);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment