Re: [PATCH] dma-buf: fix stack corruption in dma_fence_chain_release

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

By any change, did you run into this with a CTS test whose name ends 
with ".chain" ? :)

-Lionel

On 05/08/2019 10:36, Christian König wrote:
> We can't free up the chain using recursion or we run into a stack overflow.
>
> Manually free up the dangling chain nodes to avoid recursion.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/dma-buf/dma-fence-chain.c | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
> index b5089f64be2a..44a741677d25 100644
> --- a/drivers/dma-buf/dma-fence-chain.c
> +++ b/drivers/dma-buf/dma-fence-chain.c
> @@ -178,8 +178,30 @@ static bool dma_fence_chain_signaled(struct dma_fence *fence)
>   static void dma_fence_chain_release(struct dma_fence *fence)
>   {
>   	struct dma_fence_chain *chain = to_dma_fence_chain(fence);
> +	struct dma_fence *prev;
> +
> +	/* Manually unlink the chain as much as possible to avoid recursion
> +	 * and potential stack overflow.
> +	 */
> +	while ((prev = rcu_dereference_protected(chain->prev, true))) {
> +		struct dma_fence_chain *prev_chain;
> +
> +		if (kref_read(&prev->refcount) > 1)
> +		       break;
> +
> +		prev_chain = to_dma_fence_chain(prev);
> +		if (!prev_chain)
> +			break;
> +
> +		/* No need for atomic operations since we hold the last
> +		 * reference to prev_chain.
> +		 */
> +		chain->prev = prev_chain->prev;
> +		RCU_INIT_POINTER(prev_chain->prev, NULL);
> +		dma_fence_put(prev);
> +	}
> +	dma_fence_put(prev);
>   
> -	dma_fence_put(rcu_dereference_protected(chain->prev, true));
>   	dma_fence_put(chain->fence);
>   	dma_fence_free(fence);
>   }


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel