* [PATCH] NTB: ntb_transport: fix use after free
@ 2025-11-03 17:23 Baruch Siach
2025-11-03 17:45 ` Dave Jiang
2025-11-04 0:32 ` Koichiro Den
0 siblings, 2 replies; 6+ messages in thread
From: Baruch Siach @ 2025-11-03 17:23 UTC (permalink / raw)
To: Jon Mason, Dave Jiang, Allen Hubbe; +Cc: Koichiro Den, ntb, Baruch Siach
Don't call dmaengine_unmap_put() twice for the same pointer. This
results in mempool_free() being called on a freed element.
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] NTB: ntb_transport: fix use after free 2025-11-03 17:23 [PATCH] NTB: ntb_transport: fix use after free Baruch Siach @ 2025-11-03 17:45 ` Dave Jiang 2025-11-04 0:32 ` Koichiro Den 1 sibling, 0 replies; 6+ messages in thread From: Dave Jiang @ 2025-11-03 17:45 UTC (permalink / raw) To: Baruch Siach, Jon Mason, Allen Hubbe; +Cc: Koichiro Den, ntb On 11/3/25 10:23 AM, 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. > > Fixes: 6f57fd0578df ("NTB: convert to dmaengine_unmap_data") > Signed-off-by: Baruch Siach <baruch@tkos.co.il> Reviewed-by: Dave Jiang <dave.jiang@intel.com>> --- > 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; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] NTB: ntb_transport: fix use after free 2025-11-03 17:23 [PATCH] NTB: ntb_transport: fix use after free Baruch Siach 2025-11-03 17:45 ` Dave Jiang @ 2025-11-04 0:32 ` Koichiro Den 2025-11-04 5:42 ` Baruch Siach 1 sibling, 1 reply; 6+ messages in thread From: Koichiro Den @ 2025-11-04 0:32 UTC (permalink / raw) To: Baruch Siach; +Cc: Jon Mason, Dave Jiang, Allen Hubbe, ntb 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. Hi, 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? -Koichiro > > 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] NTB: ntb_transport: fix use after free 2025-11-04 0:32 ` Koichiro Den @ 2025-11-04 5:42 ` Baruch Siach 2025-11-06 3:59 ` Koichiro Den 0 siblings, 1 reply; 6+ messages in thread From: Baruch Siach @ 2025-11-04 5:42 UTC (permalink / raw) To: Koichiro Den; +Cc: Jon Mason, Dave Jiang, Allen Hubbe, ntb 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 - ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] NTB: ntb_transport: fix use after free 2025-11-04 5:42 ` Baruch Siach @ 2025-11-06 3:59 ` Koichiro Den 2025-11-06 6:02 ` Baruch Siach 0 siblings, 1 reply; 6+ messages in thread From: Koichiro Den @ 2025-11-06 3:59 UTC (permalink / raw) To: Baruch Siach; +Cc: Jon Mason, Dave Jiang, Allen Hubbe, ntb On Tue, Nov 04, 2025 at 07:42:53AM +0200, Baruch Siach wrote: > 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? Apologies for the late response. The dma_descriptor_unmap() wrapper should handle that, shouldn't it? Thanks, -Koichiro > > 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 - ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] NTB: ntb_transport: fix use after free 2025-11-06 3:59 ` Koichiro Den @ 2025-11-06 6:02 ` Baruch Siach 0 siblings, 0 replies; 6+ messages in thread From: Baruch Siach @ 2025-11-06 6:02 UTC (permalink / raw) To: Koichiro Den; +Cc: Jon Mason, Dave Jiang, Allen Hubbe, ntb Hi Koichiro, On Thu, Nov 06 2025, Koichiro Den wrote: > On Tue, Nov 04, 2025 at 07:42:53AM +0200, Baruch Siach wrote: >> 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? > > Apologies for the late response. The dma_descriptor_unmap() wrapper should > handle that, shouldn't it? Probably. The dmaengine driver I'm using is dw-axi-dmac, and I see no call to dma_descriptor_unmap() there. Not sure where the bug is, but it's definitely not in the NTB scope. Thanks for your help. Sorry for the noise. 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 - -- ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il - ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-06 6:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-03 17:23 [PATCH] NTB: ntb_transport: fix use after free Baruch Siach 2025-11-03 17:45 ` Dave Jiang 2025-11-04 0:32 ` Koichiro Den 2025-11-04 5:42 ` Baruch Siach 2025-11-06 3:59 ` Koichiro Den 2025-11-06 6:02 ` Baruch Siach
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox