[PATCH net 0/4] net: ntb_netdev: Fix TX completion and error handling

[PATCH net 0/4] net: ntb_netdev: Fix TX completion and error handling

[Koichiro Den]

Hi,

This small series fixes several TX buffer ownership and queue handling
bugs in ntb_netdev and ntb_transport.

Patch 4 first appeared in my "NTB: Add direct TX/RX using PCI endpoint
DMA" series. Sashiko later reported the same pre-existing leak while
reviewing another series, so I moved the fix here. See:
https://lore.kernel.org/r/20260815032932.151F11F000E9@smtp.kernel.org/

The ntb_transport fixes affect buffer ownership and queue handling in
ntb_netdev, the only in-tree ntb_transport_client implementation, so the
patches need to go in together. I am targeting the net tree for the
series.

Dave and other NTB maintainers, I would appreciate your review of the
ntb_transport-specific changes.

Best regards,
Koichiro


Koichiro Den (4):
  NTB: ntb_transport: Recycle TX entries before client callbacks
  net: ntb_netdev: Fix TX busy and drop handling
  NTB: ntb_transport: Fail TX enqueue when the QP link is down
  NTB: ntb_transport: Reject oversized TX buffers

 drivers/net/ntb_netdev.c    | 27 ++++++++++++++++++--------
 drivers/ntb/ntb_transport.c | 38 ++++++++++++++++++-------------------
 2 files changed, 37 insertions(+), 28 deletions(-)

base-commit: 24ef02f934eeb48830cff6b739abc3c62b1d107b
-- 
2.51.0

[PATCH net 1/4] NTB: ntb_transport: Recycle TX entries before client callbacks

[Koichiro Den]

ntb_tx_copy_callback() invokes the client callback before returning the
entry to tx_free_q. The callback may wake a stopped client queue, only
for the next enqueue to find no local entry and return -EBUSY. The window
is narrow, but the retry is unnecessary.

Save the callback data and length, then return the entry to tx_free_q
before invoking the client. A completion callback then means both the
client buffer and transport entry are ready for reuse.

Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/ntb/ntb_transport.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index f59f926d4bfa..e92b96f4afad 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1719,9 +1719,16 @@ static void ntb_transport_rxc_db(unsigned long data)
 static void ntb_tx_copy_callback(void *data,
 				 const struct dmaengine_result *res)
 {
+	struct ntb_payload_header __iomem *hdr;
 	struct ntb_queue_entry *entry = data;
-	struct ntb_transport_qp *qp = entry->qp;
-	struct ntb_payload_header __iomem *hdr = entry->tx_hdr;
+	struct ntb_transport_qp *qp;
+	unsigned int len;
+	void *cb_data;
+
+	qp = entry->qp;
+	hdr = entry->tx_hdr;
+	cb_data = entry->cb_data;
+	len = entry->len;
 
 	/* we need to check DMA results if we are using DMA */
 	if (res) {
@@ -1768,15 +1775,13 @@ static void ntb_tx_copy_callback(void *data,
 	 * "link down" or similar.  Since no payload is being sent in these
 	 * cases, there is nothing to add to the completion queue.
 	 */
-	if (entry->len > 0) {
-		qp->tx_bytes += entry->len;
-
-		if (qp->tx_handler)
-			qp->tx_handler(qp, qp->cb_data, entry->cb_data,
-				       entry->len);
-	}
+	if (len > 0)
+		qp->tx_bytes += len;
 
 	ntb_list_add(&qp->ntb_tx_free_q_lock, &entry->entry, &qp->tx_free_q);
+
+	if (len > 0 && qp->tx_handler)
+		qp->tx_handler(qp, qp->cb_data, cb_data, len);
 }
 
 static void ntb_memcpy_tx_on_stack(struct ntb_queue_entry *entry, void __iomem *offset)
-- 
2.51.0

Re: [PATCH net 1/4] NTB: ntb_transport: Recycle TX entries before client callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Memory leak of client data buffers (e.g., network sk_buff) in ntb_process_tx() error path.
- [High] Uninitialized variable qp->last_cookie on the TX DMA path defeats DMA synchronization during queue teardown.
- [High] Pending TX queue entries and their associated payloads are permanently leaked when the DMA engine is terminated.
- [High] TOCTOU race on qp->tx_handler and qp->cb_data leads to a NULL pointer dereference.
--

--- Patch [1]: [PATCH net 1/4] NTB: ntb_transport: Recycle TX entries before client callbacks ---
commit 48d0145779a008cdeb1d647661138f0bd59414bb
Author: Koichiro Den <den@valinux.co.jp>

NTB: ntb_transport: Recycle TX entries before client callbacks

ntb_tx_copy_callback() invokes the client callback before returning the
entry to tx_free_q. The callback may wake a stopped client queue, only
for the next enqueue to find no local entry and return -EBUSY. The window
is narrow, but the retry is unnecessary.

Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index f59f926d4bfaa..e92b96f4afad8 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
[ ... ]
> @@ -1768,15 +1775,13 @@ static void ntb_tx_copy_callback(void *data,
>  	 * "link down" or similar.  Since no payload is being sent in these
>  	 * cases, there is nothing to add to the completion queue.
>  	 */
> -	if (entry->len > 0) {
> -		qp->tx_bytes += entry->len;
> -
> -		if (qp->tx_handler)
> -			qp->tx_handler(qp, qp->cb_data, entry->cb_data,
> -				       entry->len);
> -	}
> +	if (len > 0)
> +		qp->tx_bytes += len;
>  
>  	ntb_list_add(&qp->ntb_tx_free_q_lock, &entry->entry, &qp->tx_free_q);
> +
> +	if (len > 0 && qp->tx_handler)
> +		qp->tx_handler(qp, qp->cb_data, cb_data, len);

[Severity: High]
This is a pre-existing issue, but does this introduce a race condition leading
to a null pointer dereference?

In ntb_tx_copy_callback(), this code checks if qp->tx_handler is non-null
before calling it:

drivers/ntb/ntb_transport.c:ntb_tx_copy_callback() {
    ...
    if (len > 0 && qp->tx_handler)
        qp->tx_handler(qp, qp->cb_data, cb_data, len);
    ...
}

Concurrently, ntb_transport_free_queue() blindly zeroes qp->tx_handler and
qp->cb_data:

drivers/ntb/ntb_transport.c:ntb_transport_free_queue() {
    ...
    qp->cb_data = NULL;
    qp->rx_handler = NULL;
    qp->tx_handler = NULL;
    ...
}

Because there is no synchronization protecting the execution of the DMA tasklet
and the teardown clearing of these pointers, can the tasklet evaluate
qp->tx_handler as non-null, but then dereference a subsequently null
qp->tx_handler, or pass a null qp->cb_data pointer to the network callback?


[Severity: High]
This is a pre-existing issue, but does ntb_process_tx() leak the SKB?

If a transmitted packet length exceeds the maximum frame size, ntb_process_tx()
invokes qp->tx_handler() with NULL instead of the original packet buffer:

drivers/ntb/ntb_transport.c:ntb_process_tx() {
    ...
    if (entry->len > qp->tx_max_frame - sizeof(struct ntb_payload_header)) {
        if (qp->tx_handler)
            qp->tx_handler(qp, qp->cb_data, NULL, -EIO);
    ...
}

Because the network callback exits early if data is NULL, it fails to free the
SKB:

drivers/net/ntb_netdev.c:ntb_netdev_tx_handler() {
    ...
    skb = data;
    if (!skb || !ndev)
        return;
    ...
}

Furthermore, ntb_process_tx() returns 0 to the caller, preventing the network
stack from freeing it on failure.


[Severity: High]
This is a pre-existing issue, but does the uninitialized qp->last_cookie defeat
DMA synchronization during queue teardown?

ntb_transport_free_queue() attempts to wait for pending TX DMA transactions:

drivers/ntb/ntb_transport.c:ntb_transport_free_queue() {
    ...
    dma_sync_wait(chan, qp->last_cookie);
    ...
}

However, ntb_async_tx_submit() drops the DMA cookie instead of assigning it to
qp->last_cookie:

drivers/ntb/ntb_transport.c:ntb_async_tx_submit() {
    ...
    cookie = dmaengine_submit(txd);
    if (dma_submit_error(cookie))
        goto err_set_unmap;

    dmaengine_unmap_put(unmap);
    dma_async_issue_pending(chan);
    return 0;
    ...
}

Could the TX synchronization wait evaluate either 0 or an unrelated RX cookie on
the TX DMA channel, leading to premature termination of in-flight TX DMA
transfers?


[Severity: High]
This is a pre-existing issue, but are pending TX queue entries and their
associated payloads permanently leaked when the DMA engine is terminated?

During queue teardown, dmaengine_terminate_all() is called:

drivers/ntb/ntb_transport.c:ntb_transport_free_queue() {
    ...
    dmaengine_terminate_all(chan);
    ...
}

This aborts any pending or active DMA descriptors. The driver relies entirely
on DMA completion callbacks to return entries to qp->tx_free_q and to invoke
the client network teardown logic.

The teardown routine then only cleans up entries present in the idle lists:

drivers/ntb/ntb_transport.c:ntb_transport_free_queue() {
    ...
    while ((entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
        kfree(entry);

    while ((entry = ntb_list_rm(&qp->ntb_tx_offl_q_lock, &qp->tx_offl_q)))
        kfree(entry);
    ...
}

Will this permanently leak active entries and their corresponding client payloads
if transactions are in-flight during module unload or link down events?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817053519.4135287-1-den@valinux.co.jp?part=1

[PATCH net 2/4] net: ntb_netdev: Fix TX busy and drop handling

[Koichiro Den]

Currently, ntb_netdev returns NETDEV_TX_BUSY for every enqueue error. It
also increments the drop and error counters while leaving the skb owned
by the qdisc, and may return BUSY with the subqueue still awake.
Retrying a permanent error cannot succeed either.

The unconditional BUSY return and premature accounting date back to the
initial driver. The error-path queue stop was later removed without
changing that return value. The current flow-control code includes a
resource check, but ntb_netdev does not honor its result before enqueue.

Honor the resource check before enqueue. For -EAGAIN and -EBUSY, stop
the subqueue, arm the existing reaper timer, and return BUSY without
touching the skb. For other errors, free the skb, increment tx_dropped,
and return NETDEV_TX_OK.

Fixes: 548c237c0a99 ("net: Add support for NTB virtual ethernet device")
Fixes: d723485cb4ca ("ntb_netdev: remove tx timeout")
Fixes: e74bfeedad08 ("NTB: Add flow control to the ntb_netdev")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/net/ntb_netdev.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 029a4a532a10..02b35cf53a62 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -199,8 +199,10 @@ static int __ntb_netdev_maybe_stop_tx(struct net_device *netdev,
 static int ntb_netdev_maybe_stop_tx(struct net_device *ndev,
 				    struct ntb_netdev_queue *q, int size)
 {
-	if (__netif_subqueue_stopped(ndev, q->qid) ||
-	    (ntb_transport_tx_free_entry(q->qp) >= size))
+	if (__netif_subqueue_stopped(ndev, q->qid))
+		return -EBUSY;
+
+	if (ntb_transport_tx_free_entry(q->qp) >= size)
 		return 0;
 
 	return __ntb_netdev_maybe_stop_tx(ndev, q, size);
@@ -256,21 +258,30 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
 
 	q = &dev->queues[qid];
 
-	ntb_netdev_maybe_stop_tx(ndev, q, tx_stop);
+	if (unlikely(ntb_netdev_maybe_stop_tx(ndev, q, tx_stop)))
+		return NETDEV_TX_BUSY;
 
 	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len);
-	if (rc)
-		goto err;
+	if (rc) {
+		if (rc == -EAGAIN || rc == -EBUSY) {
+			netif_stop_subqueue(ndev, q->qid);
+			mod_timer(&q->tx_timer,
+				  jiffies + usecs_to_jiffies(tx_time));
+			return NETDEV_TX_BUSY;
+		}
+
+		goto drop;
+	}
 
 	/* check for next submit */
 	ntb_netdev_maybe_stop_tx(ndev, q, tx_stop);
 
 	return NETDEV_TX_OK;
 
-err:
+drop:
+	dev_kfree_skb_any(skb);
 	ndev->stats.tx_dropped++;
-	ndev->stats.tx_errors++;
-	return NETDEV_TX_BUSY;
+	return NETDEV_TX_OK;
 }
 
 static void ntb_netdev_tx_timer(struct timer_list *t)
-- 
2.51.0

[PATCH net 3/4] NTB: ntb_transport: Fail TX enqueue when the QP link is down

[Koichiro Den]

Commit f195a1a6fe41 ("ntb: Drop packets when qp link is down") meant to
make ntb_transport_tx_enqueue() drop packets submitted while the QP link
is down, but it only returns 0 without consuming the packet. Zero means
success by this function's contract, so ntb_netdev reports NETDEV_TX_OK
and forgets the skb: nothing queued it, nothing frees it, and it leaks,
one skb for every transmit racing a link-down.

Return -ENOLINK instead, restoring the contract that a non-zero return
leaves the buffer owned by the caller. With the preceding patch,
ntb_netdev frees the skb on non-retryable enqueue failures and returns
NETDEV_TX_OK, so a packet racing with link-down is dropped without leaking
or entering a busy retry loop.

Fixes: f195a1a6fe41 ("ntb: Drop packets when qp link is down")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/ntb/ntb_transport.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index e92b96f4afad..4bdd81edcb87 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -2353,9 +2353,8 @@ int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
 	if (!qp || !len)
 		return -EINVAL;
 
-	/* If the qp link is down already, just ignore. */
 	if (!qp->link_is_up)
-		return 0;
+		return -ENOLINK;
 
 	entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q);
 	if (!entry) {
-- 
2.51.0

Re: [PATCH net 3/4] NTB: ntb_transport: Fail TX enqueue when the QP link is down

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Data race and state corruption during concurrent link down and packet transmission.
- [Medium] Silent memory leak of SKBs when the QP link transitions down.
--

--- Patch [3]: [PATCH net 3/4] NTB: ntb_transport: Fail TX enqueue when the QP link is down ---
commit d31ae522b81066694c3712e62ed7ae6b70c6a06a
Author: Koichiro Den <den@valinux.co.jp>

NTB: ntb_transport: Fail TX enqueue when the QP link is down

Commit f195a1a6fe41 meant to make ntb_transport_tx_enqueue() drop packets
submitted while the qp link is down, but it only returns 0 without
consuming the packet. This patch returns an error instead, restoring the
contract that a non-zero return leaves the buffer owned by the caller.

Fixes: f195a1a6fe41 ("ntb: Drop packets when qp link is down")
Cc: stable@vger.kernel.org

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index e92b96f4afad8..4bdd81edcb87a 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -2353,9 +2353,8 @@ int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
>  	if (!qp || !len)
>  		return -EINVAL;
>  
> -	/* If the qp link is down already, just ignore. */
>  	if (!qp->link_is_up)
> -		return 0;
> +		return -ENOLINK;

[Severity: High]
This isn't a bug introduced by this patch, but does the lockless qp reset in
ntb_transport_link_down() race with concurrent packet transmission?

If an administrator changes the mtu, ntb_netdev_change_mtu() calls
ntb_transport_link_down() without first stopping the tx queues via
netif_tx_disable():

drivers/net/ntb_netdev.c:ntb_netdev_change_mtu() {
	/* Bring down the link and dispose of posted rx entries */
	for (q = 0; q < dev->num_queues; q++)
		ntb_transport_link_down(dev->queues[q].qp);
}

Concurrently, the network stack can call ntb_netdev_start_xmit() and reach
ntb_transport_tx_enqueue(), which processes tx entries.
Meanwhile, ntb_transport_link_down() triggers ntb_send_link_down() and
ntb_qp_link_down_reset(), which resets qp->tx_index to 0 locklessly.

drivers/ntb/ntb_transport.c:ntb_send_link_down() {
	rc = ntb_process_tx(qp, entry);
	...
	ntb_qp_link_down_reset(qp);
}

Since ntb_async_tx() writes to tx_index without synchronization:

drivers/ntb/ntb_transport.c:ntb_async_tx() {
	WRITE_ONCE(qp->tx_index, (qp->tx_index + 1) % qp->tx_max_entry);
}

Could this lead to concurrent modifications of qp->tx_index, qp->tx_pkts, and
hardware io memory window corruption?

[Severity: Medium]
This is a pre-existing issue, but does dropping the client_ready flag in
ntb_transport_link_down() cause a silent memory leak of skb objects?

When the interface link goes down, ntb_transport_link_down() disables the
client:

drivers/ntb/ntb_transport.c:ntb_transport_link_down() {
	qp->client_ready = false;
}

Later, the receive tasklet ntb_complete_rxc() processes dma completions from
rx_post_q. Since client_ready is false, it skips calling the network receive
handler that would normally free the skb:

drivers/ntb/ntb_transport.c:ntb_complete_rxc() {
		if (qp->rx_handler && qp->client_ready)
			qp->rx_handler(qp, qp->cb_data, cb_data, len);
}

The cleanup function only drains rx_pend_q:

drivers/ntb/ntb_transport.c:ntb_transport_rx_remove() {
	entry = ntb_list_rm(&qp->ntb_rx_q_lock, &qp->rx_pend_q);
}

Does this leave skb objects in rx_post_q stranded without being freed?

>  
>  	entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q);
>  	if (!entry) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817053519.4135287-1-den@valinux.co.jp?part=3

[PATCH net 4/4] NTB: ntb_transport: Reject oversized TX buffers

[Koichiro Den]

ntb_process_tx() handles an oversized buffer by calling tx_handler()
with a NULL data pointer and returning success. ntb_netdev therefore
neither frees the skb in its completion callback nor takes its enqueue
error path, leaking it.

Reject oversized buffers in ntb_transport_tx_enqueue() before acquiring
a queue entry and return -EMSGSIZE. The caller retains ownership of the
buffer, and the preceding netdev patch frees the skb when enqueue
returns this permanent error.

Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Previously posted at:
https://lore.kernel.org/r/20260810165136.2292436-3-den@valinux.co.jp/

I moved the fix here because it addresses a pre-existing issue
independently of that feature series. I also moved the size check before
the tx_free_q lookup so -EBUSY cannot mask -EMSGSIZE.
---
 drivers/ntb/ntb_transport.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 4bdd81edcb87..f9caa1a653c5 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1955,15 +1955,6 @@ static int ntb_process_tx(struct ntb_transport_qp *qp,
 		return -EAGAIN;
 	}
 
-	if (entry->len > qp->tx_max_frame - sizeof(struct ntb_payload_header)) {
-		if (qp->tx_handler)
-			qp->tx_handler(qp, qp->cb_data, NULL, -EIO);
-
-		ntb_list_add(&qp->ntb_tx_free_q_lock, &entry->entry,
-			     &qp->tx_free_q);
-		return 0;
-	}
-
 	ntb_async_tx(qp, entry);
 
 	qp->tx_pkts++;
@@ -2356,6 +2347,9 @@ int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
 	if (!qp->link_is_up)
 		return -ENOLINK;
 
+	if (len > qp->tx_max_frame - sizeof(struct ntb_payload_header))
+		return -EMSGSIZE;
+
 	entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q);
 	if (!entry) {
 		qp->tx_err_no_buf++;
-- 
2.51.0

Re: [PATCH net 4/4] NTB: ntb_transport: Reject oversized TX buffers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Unvalidated `transport_mtu` module parameter allows division by zero and integer underflows leading to out-of-bounds memory writes.
- [High] Queue entries are leaked in `ntb_send_link_down()` when the transmission ring is full.
- [High] Queue teardown permanently leaks `ntb_queue_entry` objects and associated network payload buffers.
--

--- Patch [4]: [PATCH net 4/4] NTB: ntb_transport: Reject oversized TX buffers ---
commit 0674c1f1ad07dc65b6ed7cc1cb352ca787323a36
Author: Koichiro Den <den@valinux.co.jp>
Subject: NTB: ntb_transport: Reject oversized TX buffers

This commit moves the oversized buffer check from ntb_process_tx() to 
ntb_transport_tx_enqueue(). It returns -EMSGSIZE before acquiring a queue
entry, allowing the caller to retain ownership and free the buffer.

Link: https://lore.kernel.org/r/20260810165136.2292436-3-den@valinux.co.jp/

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 4bdd81edcb87a..f9caa1a653c5b 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1955,15 +1955,6 @@ static int ntb_process_tx(struct ntb_transport_qp *qp,
>  		return -EAGAIN;
>  	}
>  
> -	if (entry->len > qp->tx_max_frame - sizeof(struct ntb_payload_header)) {
> -		if (qp->tx_handler)
> -			qp->tx_handler(qp, qp->cb_data, NULL, -EIO);
> -
> -		ntb_list_add(&qp->ntb_tx_free_q_lock, &entry->entry,
> -			     &qp->tx_free_q);
> -		return 0;
> -	}
> -
>  	ntb_async_tx(qp, entry);

[Severity: High]
This is a pre-existing issue, but does ntb_send_link_down() leak queue entries
when ntb_process_tx() fails?

When ntb_process_tx() returns an error like -EAGAIN,
ntb_transport_tx_enqueue() correctly adds the entry back to tx_free_q.
However, ntb_send_link_down() doesn't seem to return the entry to the
queue or free it:

drivers/ntb/ntb_transport.c:ntb_send_link_down() {
    ...
    rc = ntb_process_tx(qp, entry);
    if (rc)
        dev_err(&pdev->dev, "ntb: QP%d unable to send linkdown msg\n",
                qp->qp_num);
    ...
}

Could repeated link flaps permanently exhaust the queue?

[ ... ]

> @@ -2356,6 +2347,9 @@ int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
>  	if (!qp->link_is_up)
>  		return -ENOLINK;
>  
> +	if (len > qp->tx_max_frame - sizeof(struct ntb_payload_header))
> +		return -EMSGSIZE;

[Severity: High]
This is a pre-existing issue, but if a user sets the transport_mtu module
parameter to a small value (like 1 to 11), could this length check underflow?

If qp->tx_max_frame evaluates to less than sizeof(struct ntb_payload_header),
the subtraction will underflow to a large positive integer, bypassing the
bounds check. This would then lead to out-of-bounds memory writes during
pointer arithmetic in ntb_async_tx():

drivers/ntb/ntb_transport.c:ntb_async_tx() {
    ...
    hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
    ...
}

Also, if transport_mtu is set to 0, it appears to cause a division by zero in
ntb_transport_init_queue():

drivers/ntb/ntb_transport.c:ntb_transport_init_queue() {
    ...
    qp->tx_max_entry = tx_size / qp->tx_max_frame;
    ...
}

Should the module parameter be validated to prevent this?

> +
>  	entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q);
>  	if (!entry) {
>  		qp->tx_err_no_buf++;

[Severity: High]
This is a pre-existing issue, but does queue teardown permanently leak
ntb_queue_entry objects and their associated network buffers?

In ntb_transport_free_queue(), entries are freed without invoking their
completion handlers (like tx_handler) to release the attached network
buffers:

drivers/ntb/ntb_transport.c:ntb_transport_free_queue() {
    ...
    while ((entry = ntb_list_rm(&qp->ntb_tx_offl_q_lock, &qp->tx_offl_q)))
        kfree(entry);
    ...
}

Additionally, it seems DMA operations are terminated without callbacks:

drivers/ntb/ntb_transport.c:ntb_transport_free_queue() {
    ...
    dmaengine_terminate_all(chan);
    ...
}

Because ntb_async_tx_submit() doesn't populate qp->last_cookie with the
TX cookie, does the preceding dma_sync_wait() incorrectly wait on the RX
cookie, allowing engine termination to orphan all active TX entries?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817053519.4135287-1-den@valinux.co.jp?part=4