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 onto to use the now freed datastructure.
In particular, we get struct by commit 0e2f733addbf ("dma-buf: make dma_fence structure a bit smaller v2") as the cb_list is immediately clobbered by the kfree_rcu.
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+ --- drivers/dma-buf/sw_sync.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 051f6c2873c7..27b1d549ed38 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -132,17 +132,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);
Am 12.08.19 um 17:42 schrieb Chris Wilson:
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 onto to use the now freed datastructure.
In particular, we get struct by commit 0e2f733addbf ("dma-buf: make dma_fence structure a bit smaller v2") as the cb_list is immediately clobbered by the kfree_rcu.
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
drivers/dma-buf/sw_sync.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 051f6c2873c7..27b1d549ed38 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -132,17 +132,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);
linux-stable-mirror@lists.linaro.org
-
Chris Wilson -
Koenig, Christian