[PATCH] dma-buf/sw_sync: Synchronize signal vs syncpt free

[Koenig, Christian]

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 at chris-wilson.co.uk>
> Cc: Sumit Semwal <sumit.semwal at linaro.org>
> Cc: Sean Paul <seanpaul at chromium.org>
> Cc: Gustavo Padovan <gustavo at padovan.org>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: <stable at vger.kernel.org> # v4.14+

Acked-by: Christian König <christian.koenig at 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);