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

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Not even remotely :)I tested this with my own crafted code inside the 
kernel.

It's probably quite some hassle to actually trigger this problem from 
userspace and I only found it because I created a very very long 
sequence chain by accident.

Christian.

Am 05.08.19 um 14:03 schrieb Lionel Landwerlin:
> 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