Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

dma-buf/sw_sync: Synchronize signal vs syncpt free

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@chris-wilson.co.uk

+7 -9
+7 -9
drivers/dma-buf/sw_sync.c
··· 132 132 { 133 133 struct sync_pt *pt = dma_fence_to_sync_pt(fence); 134 134 struct sync_timeline *parent = dma_fence_parent(fence); 135 + unsigned long flags; 135 136 137 + spin_lock_irqsave(fence->lock, flags); 136 138 if (!list_empty(&pt->link)) { 137 - unsigned long flags; 138 - 139 - spin_lock_irqsave(fence->lock, flags); 140 - if (!list_empty(&pt->link)) { 141 - list_del(&pt->link); 142 - rb_erase(&pt->node, &parent->pt_tree); 143 - } 144 - spin_unlock_irqrestore(fence->lock, flags); 139 + list_del(&pt->link); 140 + rb_erase(&pt->node, &parent->pt_tree); 145 141 } 142 + spin_unlock_irqrestore(fence->lock, flags); 146 143 147 144 sync_timeline_put(parent); 148 145 dma_fence_free(fence); ··· 262 265 p = &parent->rb_left; 263 266 } else { 264 267 if (dma_fence_get_rcu(&other->base)) { 265 - dma_fence_put(&pt->base); 268 + sync_timeline_put(obj); 269 + kfree(pt); 266 270 pt = other; 267 271 goto unlock; 268 272 }