Re: [PATCH] NTB: ntb_transport: fix use after free

[Baruch Siach <baruch@tkos.co.il>]

Hi Koichiro,

Thanks for taking the time to review this.

On Tue, Nov 04 2025, Koichiro Den wrote:
> On Mon, Nov 03, 2025 at 07:23:19PM +0200, Baruch Siach wrote:
>> Don't call dmaengine_unmap_put() twice for the same pointer. This
>> results in mempool_free() being called on a freed element.
>
> While the second dmaengine_unmap_put() looks redundant at first glance, my
> understanding was that it compensates for the refcount decrement that would
> normally happen on the dmaengine completion path, which never occurs when
> dmaengine_submit() fails. Am I missing something?

I guess you are right. Now I see that dma_set_unmap() increments the
refcount. But I can't find the second refcount decrement in completion
path. I only see one call to dmaengine_unmap_put(). Can you enlighten
me?

Thanks,
baruch

>> Fixes: 6f57fd0578df ("NTB: convert to dmaengine_unmap_data")
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>>  drivers/ntb/ntb_transport.c | 22 +++++++++-------------
>>  1 file changed, 9 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
>> index eb875e3db2e3..809fb09658b4 100644
>> --- a/drivers/ntb/ntb_transport.c
>> +++ b/drivers/ntb/ntb_transport.c
>> @@ -1573,14 +1573,14 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
>>  	unmap->addr[0] = dma_map_page(device->dev, virt_to_page(offset),
>>  				      pay_off, len, DMA_TO_DEVICE);
>>  	if (dma_mapping_error(device->dev, unmap->addr[0]))
>> -		goto err_get_unmap;
>> +		goto err_unmap;
>>  
>>  	unmap->to_cnt = 1;
>>  
>>  	unmap->addr[1] = dma_map_page(device->dev, virt_to_page(buf),
>>  				      buff_off, len, DMA_FROM_DEVICE);
>>  	if (dma_mapping_error(device->dev, unmap->addr[1]))
>> -		goto err_get_unmap;
>> +		goto err_unmap;
>>  
>>  	unmap->from_cnt = 1;
>>  
>> @@ -1588,7 +1588,7 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
>>  					     unmap->addr[0], len,
>>  					     DMA_PREP_INTERRUPT);
>>  	if (!txd)
>> -		goto err_get_unmap;
>> +		goto err_unmap;
>>  
>>  	txd->callback_result = ntb_rx_copy_callback;
>>  	txd->callback_param = entry;
>> @@ -1596,7 +1596,7 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
>>  
>>  	cookie = dmaengine_submit(txd);
>>  	if (dma_submit_error(cookie))
>> -		goto err_set_unmap;
>> +		goto err_unmap;
>>  
>>  	dmaengine_unmap_put(unmap);
>>  
>> @@ -1606,9 +1606,7 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
>>  
>>  	return 0;
>>  
>> -err_set_unmap:
>> -	dmaengine_unmap_put(unmap);
>> -err_get_unmap:
>> +err_unmap:
>>  	dmaengine_unmap_put(unmap);
>>  err:
>>  	return -ENXIO;
>> @@ -1854,14 +1852,14 @@ static int ntb_async_tx_submit(struct ntb_transport_qp *qp,
>>  	unmap->addr[0] = dma_map_page(device->dev, virt_to_page(buf),
>>  				      buff_off, len, DMA_TO_DEVICE);
>>  	if (dma_mapping_error(device->dev, unmap->addr[0]))
>> -		goto err_get_unmap;
>> +		goto err_unmap;
>>  
>>  	unmap->to_cnt = 1;
>>  
>>  	txd = device->device_prep_dma_memcpy(chan, dest, unmap->addr[0], len,
>>  					     DMA_PREP_INTERRUPT);
>>  	if (!txd)
>> -		goto err_get_unmap;
>> +		goto err_unmap;
>>  
>>  	txd->callback_result = ntb_tx_copy_callback;
>>  	txd->callback_param = entry;
>> @@ -1869,16 +1867,14 @@ static int ntb_async_tx_submit(struct ntb_transport_qp *qp,
>>  
>>  	cookie = dmaengine_submit(txd);
>>  	if (dma_submit_error(cookie))
>> -		goto err_set_unmap;
>> +		goto err_unmap;
>>  
>>  	dmaengine_unmap_put(unmap);
>>  
>>  	dma_async_issue_pending(chan);
>>  
>>  	return 0;
>> -err_set_unmap:
>> -	dmaengine_unmap_put(unmap);
>> -err_get_unmap:
>> +err_unmap:
>>  	dmaengine_unmap_put(unmap);
>>  err:
>>  	return -ENXIO;
>> -- 
>> 2.51.0
>> 

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -