Re: [PATCH 2/2] dmaengine: qcom_hidma: fix handoff FIFO memory leak on driver removal

[Sinan Kaya <Okaya@kernel.org>]

On 6/5/2025 9:04 AM, Eugen Hristev wrote:
>> diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
>> index fee448499777..0c2bae46746c 100644
>> --- a/drivers/dma/qcom/hidma_ll.c
>> +++ b/drivers/dma/qcom/hidma_ll.c
>> @@ -816,6 +816,7 @@ int hidma_ll_uninit(struct hidma_lldev *lldev)
>>   
>>   	required_bytes = sizeof(struct hidma_tre) * lldev->nr_tres;
>>   	tasklet_kill(&lldev->task);
>> +	kfifo_free(&lldev->handoff_fifo);
>>   	memset(lldev->trepool, 0, required_bytes);
>>   	lldev->trepool = NULL;
>>   	atomic_set(&lldev->pending_tre_count, 0);
> Is it possible that the handoff_fifo is freed, then we could observe
> reset complete interrupts before they are being cleared in
> hidma_ll_uninit later on, which would lead to the following call chain
>
>   hidma_ll_inthandler - hidma_ll_int_handler_internal -
> hidma_handle_tre_completion - hidma_post_completed -
> tasklet_schedule(&lldev->task); - hidma_ll_tre_complete - kfifo_out

According to the documentation, the way to guarantee this from not happening

is to call tasklet_disable() to ensure that tasklet completes execution. 
Only after that

data structures used by the tasklet can be freed.

I think proper order is:

1. tasklet_disable

2. tasklet_kill

3. kfifo_free