Re: [PATCH] net: ns83820: check DMA mapping errors in hard_start_xmit
From: Paolo Abeni <pabeni@redhat.com>
Date: 2026-03-31 10:05:26
Also in:
lkml, stable
On 3/27/26 2:16 AM, Wang Jun wrote:
The ns83820 driver currently ignores the return values of dma_map_single()
and skb_frag_dma_map() in the transmit path. If DMA mapping fails due to
IOMMU exhaustion or SWIOTLB pressure, the driver may proceed with invalid
DMA addresses, potentially causing hardware errors, data corruption, or
system instability.
Additionally, if mapping fails midway through processing fragmented
packets,previously mapped DMA resources are not released, leading
to DMA resource leaks.
Fix this by:
1. Checking dma_mapping_error() after each DMA mapping call.
2. Implementing an error handling path to unmap successfully
mapped buffers(both linear and fragments) using
dma_unmap_single().
3. Freeing the skb using dev_kfree_skb_any() to safely handle
both process and softirq contexts, as hard_start_xmit may be
called with IRQs enabled.
4. Returning NETDEV_TX_OK to drop the packet gracefully and
prevent TX queue stagnation.
This ensures compliance with the DMA API guidelines and
improves driver stability under memory pressure.
Fixes: fd9e4d6fec15 ("natsemi: switch from 'pci_' to 'dma_' API")The fix tag looks wrong, as the above just to mechanical substitution of used APIs. The issue is pre-existent
quoted hunk ↗ jump to hunk
Cc: stable@vger.kernel.org Signed-off-by: Wang Jun <redacted> --- drivers/net/ethernet/natsemi/ns83820.c | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c index cdbf82affa7b..90465e4977c3 100644 --- a/drivers/net/ethernet/natsemi/ns83820.c +++ b/drivers/net/ethernet/natsemi/ns83820.c@@ -1051,6 +1051,12 @@ static netdev_tx_t ns83820_hard_start_xmit(struct sk_buff *skb, int stopped = 0; int do_intr = 0; volatile __le32 *first_desc; + dma_addr_t frag_dma_addr[MAX_SKB_FRAGS]; + unsigned int frag_dma_len[MAX_SKB_FRAGS]; + int frag_mapped_count = 0; + dma_addr_t main_buf = 0; + unsigned int main_len = 0; + int i;
Please respect the reverse christmas tree order above.
quoted hunk ↗ jump to hunk
dprintk("ns83820_hard_start_xmit\n");@@ -1121,6 +1127,13 @@ static netdev_tx_t ns83820_hard_start_xmit(struct sk_buff *skb, buf = dma_map_single(&dev->pci_dev->dev, skb->data, len, DMA_TO_DEVICE); + if (dma_mapping_error(&dev->pci_dev->dev, buf)) { + dev_kfree_skb_any(skb); + return NETDEV_TX_OK; + } + main_buf = buf; + main_len = len; + first_desc = dev->tx_descs + (free_idx * DESC_SIZE); for (;;) {@@ -1144,6 +1157,15 @@ static netdev_tx_t ns83820_hard_start_xmit(struct sk_buff *skb, buf = skb_frag_dma_map(&dev->pci_dev->dev, frag, 0, skb_frag_size(frag), DMA_TO_DEVICE); + if (dma_mapping_error(&dev->pci_dev->dev, buf)) + goto dma_map_error; + + if (frag_mapped_count < MAX_SKB_FRAGS) { + frag_dma_addr[frag_mapped_count] = buf; + frag_dma_len[frag_mapped_count] = skb_frag_size(frag); + frag_mapped_count++; + } + dprintk("frag: buf=%08Lx page=%08lx offset=%08lx\n", (long long)buf, (long) page_to_pfn(frag->page), frag->page_offset);@@ -1166,6 +1188,16 @@ static netdev_tx_t ns83820_hard_start_xmit(struct sk_buff *skb, if (stopped && (dev->tx_done_idx != tx_done_idx) && start_tx_okay(dev)) netif_start_queue(ndev); + return NETDEV_TX_OK; +dma_map_error: + dma_unmap_single(&dev->pci_dev->dev, main_buf, main_len, DMA_TO_DEVICE); + for (i = 0; i < frag_mapped_count; i++) { + dma_unmap_single(&dev->pci_dev->dev, frag_dma_addr[i], frag_dma_len[i], + DMA_TO_DEVICE); + } + + dev_kfree_skb_any(skb);
AI review says: If nr_free < MIN_TX_DESC_FREE earlier in the function, netif_stop_queue is called and the stopped flag is set to 1. By returning NETDEV_TX_OK directly from the error path, we skip the race check at the end of the normal flow: if (stopped && (dev->tx_done_idx != tx_done_idx) && start_tx_okay(dev)) netif_start_queue(ndev); If all pending packets completed concurrently right before the queue was stopped, could this cause the TX queue to stall indefinitely until the tx watchdog fires?