From: Chris Wilson chris@chris-wilson.co.uk
[ Upstream commit d3c6dd1fb30d3853c2012549affe75c930f4a2f9 ]
During release of the syncpt, we remove it from the list of syncpt and the tree, but only if it is not already been removed. However, during signaling, we first remove the syncpt from the list. So, if we concurrently free and signal the syncpt, the free may decide that it is not part of the tree and immediately free itself -- meanwhile the signaler goes on to use the now freed datastructure.
In particular, we get struck by commit 0e2f733addbf ("dma-buf: make dma_fence structure a bit smaller v2") as the cb_list is immediately clobbered by the kfree_rcu.
v2: Avoid calling into timeline_fence_release() from under the spinlock
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111381 Fixes: d3862e44daa7 ("dma-buf/sw-sync: Fix locking around sync_timeline lists") References: 0e2f733addbf ("dma-buf: make dma_fence structure a bit smaller v2") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Sean Paul seanpaul@chromium.org Cc: Gustavo Padovan gustavo@padovan.org Cc: Christian König christian.koenig@amd.com Cc: stable@vger.kernel.org # v4.14+ Acked-by: Christian König christian.koenig@amd.com Link: https://patchwork.freedesktop.org/patch/msgid/20190812154247.20508-1-chris@c... Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/dma-buf/sw_sync.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 53c1d6d36a642..81ba4eb348909 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -141,17 +141,14 @@ static void timeline_fence_release(struct dma_fence *fence) { struct sync_pt *pt = dma_fence_to_sync_pt(fence); struct sync_timeline *parent = dma_fence_parent(fence); + unsigned long flags;
+ spin_lock_irqsave(fence->lock, flags); if (!list_empty(&pt->link)) { - unsigned long flags; - - spin_lock_irqsave(fence->lock, flags); - if (!list_empty(&pt->link)) { - list_del(&pt->link); - rb_erase(&pt->node, &parent->pt_tree); - } - spin_unlock_irqrestore(fence->lock, flags); + list_del(&pt->link); + rb_erase(&pt->node, &parent->pt_tree); } + spin_unlock_irqrestore(fence->lock, flags);
sync_timeline_put(parent); dma_fence_free(fence); @@ -274,7 +271,8 @@ static struct sync_pt *sync_pt_create(struct sync_timeline *obj, p = &parent->rb_left; } else { if (dma_fence_get_rcu(&other->base)) { - dma_fence_put(&pt->base); + sync_timeline_put(obj); + kfree(pt); pt = other; goto unlock; }