[PATCH] dmaengine: sun6i-dma: Fix use-after-free in error handling paths

[Hongling Zeng]

   Hi Jernej,

   Thanks for the feedback. I've sent v2 implementing your suggestion - 
refactored to use a helper function sun6i_dma_free_lli_list() that accepts
   struct sun6i_desc * parameter, eliminating code duplication.

   Please help review.

   Best regards,
   Hongling Zeng


在 2026年06月13日 15:46, Jernej Škrabec 写道:
> Dne četrtek, 11. junij 2026 ob 08:36:31 Srednjeevropski poletni čas je Hongling Zeng napisal(a):
>> In error handling paths, the for loop frees v_lli in the loop body,
>> then accesses v_lli->v_lli_next and v_lli->p_lli_next in the
>> increment expression, which is use-after-free.
>>
>> Fix by saving both the next virtual and physical pointers before
>> freeing the current node.
>>
>> Fixes: 555859308723 ("dmaengine: Add driver for Allwinner sun6i DMA")
>> Signed-off-by: Hongling Zeng <zenghongling at kylinos.cn>
>> ---
>>   drivers/dma/sun6i-dma.c | 20 ++++++++++++++++----
>>   1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
>> index a9a254dbf8cb..eb9c4ae87ac8 100644
>> --- a/drivers/dma/sun6i-dma.c
>> +++ b/drivers/dma/sun6i-dma.c
>> @@ -788,9 +788,15 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
>>   	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
>>   
>>   err_lli_free:
>> -	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
>> -	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
>> +	p_lli = txd->p_lli;
>> +	v_lli = txd->v_lli;
>> +	while (v_lli) {
>> +		struct sun6i_dma_lli *next_v_lli = v_lli->v_lli_next;
>> +		dma_addr_t next_p_lli = v_lli->p_lli_next;
>>   		dma_pool_free(sdev->pool, v_lli, p_lli);
>> +		v_lli = next_v_lli;
>> +		p_lli = next_p_lli;
>> +	}
>>   	kfree(txd);
>>   	return NULL;
>>   }
>> @@ -869,9 +875,15 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
>>   	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
>>   
>>   err_lli_free:
>> -	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
>> -	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
>> +	p_lli = txd->p_lli;
>> +	v_lli = txd->v_lli;
>> +	while (v_lli) {
>> +		struct sun6i_dma_lli *next_v_lli = v_lli->v_lli_next;
>> +		dma_addr_t next_p_lli = v_lli->p_lli_next;
>>   		dma_pool_free(sdev->pool, v_lli, p_lli);
>> +		v_lli = next_v_lli;
>> +		p_lli = next_p_lli;
>> +	}
>>   	kfree(txd);
>>   	return NULL;
>>   }
>>
> This is certainly a valid fix, but it's replicating what sun6i_dma_free_desc()
> is already doing. It would be better to split code to accept struct sun6i_desc
> *txd parameter and call that instead from all places.
>
> Best regards,
> Jernej
>