Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 net] bnxt_en: fix DMA mapping length for padded small packets
@ 2026-10-06  4:21 Eric Dumazet
  2026-10-06  7:38 ` Michael Chan
  2026-10-07  4:22 ` netdev-bot+sashiko
  0 siblings, 2 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-10-06  4:21 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, netdev, Eric Dumazet, stable, Stefan Fleischmann,
	Eric Dumazet, Michael Chan, Pavan Chebbi, Andrew Lunn,
	Bernhard Schmidt

Stefan Fleischmann reported Intel IOMMU DMA Read faults on BCM57412
NetXtreme-E NICs when transmitting packets on VLAN/macvlan interfaces:

  DMAR: [DMA Read NO_PASID] Request device [18:00.0] fault addr 0xfc499000
        [fault reason 0x06] PTE Read access is not set
  bnxt_en 0000:18:00.0 eno1np0: Abandoning msg {0xb4 0x41a} len: 0 due to firmware status: 0x2000001
  ...
  NETDEV WATCHDOG: eno1np0 (bnxt_en): transmit queue 0 timed out

The fault address (0xfc499000) is on an exact 4KB page boundary,
pointing to a DMA read buffer overrun.

In bnxt_start_xmit(), packets smaller than BNXT_MIN_PKT_SIZE (52 bytes),
such as 42-byte untagged ARP frames, are padded:

    if (length < BNXT_MIN_PKT_SIZE) {
        pad = BNXT_MIN_PKT_SIZE - length;
        if (skb_pad(skb, pad))
            goto tx_kick_pending;
        length = BNXT_MIN_PKT_SIZE;
    }

    mapping = dma_map_single(&pdev->dev, skb->data, len, DMA_TO_DEVICE);
    ...
    dma_unmap_len_set(tx_buf, len, len);

However, 'len' was initialized earlier to skb_headlen(skb) (e.g. 42 bytes)
and is left unadjusted after padding. Consequently, dma_map_single() and
dma_unmap_len_set() map and track only 42 bytes.

Later, the hardware TX buffer descriptor is programmed with the padded length:

    txbd->tx_bd_len_flags_type =
        cpu_to_le32(((len + pad) << TX_BD_LEN_SHIFT) | flags |
                    TX_BD_FLAGS_PACKET_END);

The NIC DMA engine is thus instructed to read 52 bytes from a region where
only 42 bytes were DMA-mapped. If skb->data ends near the boundary of a 4KB
page (within 'pad' bytes of the next page), the hardware DMA read overruns
into the unmapped adjacent page, triggering an IOMMU fault.

This issue was exposed after commit 447cbe95ebb9 ("vlan: fix skb_under_panic
and races when toggling HW VLAN offload") because reserving extra VLAN
headroom rounded LL_RESERVED_SPACE from 48 up to 64 bytes, shifting skb->data
offsets and potentially causing small frames to land right against page
boundaries.

Fix this by using skb_put_padto(skb, BNXT_MIN_PKT_SIZE) earlier in
bnxt_start_xmit().
This ensures skb->len and skb_headlen(skb) reflect the padded size so
that dma_map_single() maps the full buffer and the descriptor length is
consistent. This also removes the temporary 'pad' variable and masking logic.

Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.")
Cc: stable@vger.kernel.org
Reported-by: Stefan Fleischmann <sfle@kth.se>
Closes: https://lore.kernel.org/netdev/20261004122616.56714cbd@nargothrond/
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Cc: Michael Chan <michael.chan@broadcom.com>
Cc: Pavan Chebbi <pavan.chebbi@broadcom.com>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: Bernhard Schmidt <berni@debian.org>
---
v2: Move skb_put_padto() earlier (Sashiko)
v1: https://lore.kernel.org/netdev/20261005023812.130639-1-edumazet@kernel.org/

 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d7728d0c5b6e63ee72de9dea54426bb4c8b7a9fc..7f379f21fe46543a7067462588522d4ab6ef4603 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -486,7 +486,7 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct netdev_queue *txq;
 	int i;
 	dma_addr_t mapping;
-	unsigned int length, pad = 0;
+	unsigned int length;
 	u32 len, free_size, vlan_tag_flags, cfa_action, flags;
 	struct bnxt_ptp_cfg *ptp = bp->ptp_cfg;
 	struct pci_dev *pdev = bp->pdev;
@@ -507,6 +507,11 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	txr = &bp->tx_ring[bp->tx_ring_map[i]];
 	prod = txr->tx_prod;
 
+	if (skb_put_padto(skb, BNXT_MIN_PKT_SIZE)) {
+		/* SKB already freed. */
+		goto tx_kick_pending;
+	}
+
 #if (MAX_SKB_FRAGS > TX_MAX_FRAGS)
 	if (skb_shinfo(skb)->nr_frags > TX_MAX_FRAGS) {
 		netdev_warn_once(dev, "SKB has too many (%d) fragments, max supported is %d.  SKB will be linearized.\n",
@@ -672,14 +677,6 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 normal_tx:
-	if (length < BNXT_MIN_PKT_SIZE) {
-		pad = BNXT_MIN_PKT_SIZE - length;
-		if (skb_pad(skb, pad))
-			/* SKB already freed. */
-			goto tx_kick_pending;
-		length = BNXT_MIN_PKT_SIZE;
-	}
-
 	mapping = dma_map_single(&pdev->dev, skb->data, len, DMA_TO_DEVICE);
 
 	if (unlikely(dma_mapping_error(&pdev->dev, mapping)))
@@ -759,10 +756,7 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		txbd->tx_bd_len_flags_type = cpu_to_le32(flags);
 	}
 
-	flags &= ~TX_BD_LEN;
-	txbd->tx_bd_len_flags_type =
-		cpu_to_le32(((len + pad) << TX_BD_LEN_SHIFT) | flags |
-			    TX_BD_FLAGS_PACKET_END);
+	txbd->tx_bd_len_flags_type |= cpu_to_le32(TX_BD_FLAGS_PACKET_END);
 
 	netdev_tx_sent_queue(txq, skb->len);
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 net] bnxt_en: fix DMA mapping length for padded small packets
  2026-10-06  4:21 [PATCH v2 net] bnxt_en: fix DMA mapping length for padded small packets Eric Dumazet
@ 2026-10-06  7:38 ` Michael Chan
  2026-10-07  4:22 ` netdev-bot+sashiko
  1 sibling, 0 replies; 4+ messages in thread
From: Michael Chan @ 2026-10-06  7:38 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	netdev, stable, Stefan Fleischmann, Eric Dumazet, Pavan Chebbi,
	Andrew Lunn, Bernhard Schmidt

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

On Mon, Oct 5, 2026 at 9:22 PM Eric Dumazet <edumazet@kernel.org> wrote:

> Fix this by using skb_put_padto(skb, BNXT_MIN_PKT_SIZE) earlier in
> bnxt_start_xmit().
> This ensures skb->len and skb_headlen(skb) reflect the padded size so
> that dma_map_single() maps the full buffer and the descriptor length is
> consistent. This also removes the temporary 'pad' variable and masking logic.
>
> Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.")
> Cc: stable@vger.kernel.org
> Reported-by: Stefan Fleischmann <sfle@kth.se>
> Closes: https://lore.kernel.org/netdev/20261004122616.56714cbd@nargothrond/
> Signed-off-by: Eric Dumazet <edumazet@google.com>

v2 looks good to me.  Thanks.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 net] bnxt_en: fix DMA mapping length for padded small packets
  2026-10-06  4:21 [PATCH v2 net] bnxt_en: fix DMA mapping length for padded small packets Eric Dumazet
  2026-10-06  7:38 ` Michael Chan
@ 2026-10-07  4:22 ` netdev-bot+sashiko
  2026-10-07  4:52   ` Eric Dumazet
  1 sibling, 1 reply; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-10-07  4:22 UTC (permalink / raw)
  To: edumazet
  Cc: davem, kuba, pabeni, horms, netdev, stable, sfle, edumazet,
	michael.chan, pavan.chebbi, andrew+netdev, berni

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

Critical: 0 · High: 0 (1) · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Regression in the software UDP GSO path.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the sister
  bnge driver has the same stale-fragment ordering bug that the bnxt v2…

--

bnxt_en: fix DMA mapping length for padded small packets

This patch moves short-frame padding in bnxt_start_xmit() to the top of
the function and uses skb_put_padto(). As a result, skb_headlen() now
includes the pad when the linear head is DMA-mapped. This fixes the IOMMU
read faults that happened when the TX BD length was larger than the
mapped length.

Closes: https://lore.kernel.org/netdev/20261004122616.56714cbd@nargothrond/

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index d7728d0c5b6e6..7f379f21fe465 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c

[ ... ]

> @@ -507,6 +507,11 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	txr = &bp->tx_ring[bp->tx_ring_map[i]];
>  	prod = txr->tx_prod;
>  
> +	if (skb_put_padto(skb, BNXT_MIN_PKT_SIZE)) {
> +		/* SKB already freed. */
> +		goto tx_kick_pending;
> +	}
> +

[Severity: Medium]
Does moving the pad ahead of the SW USO branch change what
bnxt_sw_udp_gso_xmit() puts on the wire?

Before this patch, padding only ran at normal_tx:, after the SW USO branch
had already returned. So bnxt_sw_udp_gso_xmit() always got the unpadded
skb.

Now a UDP GSO skb shorter than 52 bytes is padded first. On devices
without BNXT_FLAG_UDP_GSO_CAP it then goes here:

	if (skb_is_gso(skb) &&
	    (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) &&
	    !(bp->flags & BNXT_FLAG_UDP_GSO_CAP)) {
		int rc = bnxt_sw_udp_gso_xmit(bp, txr, txq, skb);

bnxt_sw_udp_gso_xmit() in bnxt_gso.c then computes:

	total_payload = skb->len - hdr_len;
	num_segs = DIV_ROUND_UP(total_payload, mss);

This counts the zero pad as UDP payload. tso_build_hdr() builds
iph->tot_len and the UDP length from the segment payload, and the NIC
fills in valid checksums. Would the receiver then accept the pad bytes as
real UDP data?

For example, Ethernet 14 + IPv4 20 + UDP 8 + 2 bytes of data is 44 bytes.
With gso_size=1 that is padded to 52. total_payload goes from 2 to 10, so
10 datagrams are sent instead of 2, and eight of them carry only zeros.

An unprivileged UDP socket can trigger this by setting UDP_SEGMENT to a
small gso_size and sending a few bytes over IPv4.

Also, BNXT_MIN_PKT_SIZE is a per-frame minimum. Padding the super-packet
still leaves the individual SW USO segments (for example 43 bytes)
shorter than that.

Should the pad be skipped for skb_is_gso() skbs, or applied after the SW
USO branch?

[Severity: High]
This isn't a bug introduced by this patch, but the sister bnge driver
seems to have the ordering problem that this v2 placement avoids for
bnxt. In bnge_start_xmit() in drivers/net/ethernet/broadcom/bnge/bnge_txrx.c,
the frag count is read before padding:

	last_frag = skb_shinfo(skb)->nr_frags;
	...
	tx_buf->nr_frags = last_frag;
	...
	if (eth_skb_pad(skb))
		goto tx_kick_pending;

A nonlinear skb shorter than ETH_ZLEN has skb_tailroom() of 0. In that
case __skb_pad() goes through pskb_expand_head() and skb_linearize().
That pulls the frag data into the head, drops the frag page references
and sets nr_frags to 0.

bnge_start_xmit() then keeps using the stale last_frag:

	flags = (len << TX_BD_LEN_SHIFT) | TX_BD_TYPE_LONG_TX_BD |
		TX_BD_CNT(last_frag + 2);
	...
	for (i = 0; i < last_frag; i++) {
		frag = &skb_shinfo(skb)->frags[i];
		...
		mapping = skb_frag_dma_map(bd->dev, frag, 0, len,
					   DMA_TO_DEVICE);

Can this DMA-map frag pages that the skb no longer holds a reference to?
If so, the NIC would send the frag data twice and read pages that may
already be freed or recycled, such as page_pool pages.

Two ways to get such skbs are AF_PACKET TX_RING SOCK_RAW frames and
forwarded small packets whose payload stayed in a frag after
pskb_trim_rcsum().

As a separate fix, would it make sense to call eth_skb_pad() before
nr_frags is read in bnge_start_xmit(), the same way this patch does for
bnxt?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006042153.199444-1-edumazet%40kernel.org

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 net] bnxt_en: fix DMA mapping length for padded small packets
  2026-10-07  4:22 ` netdev-bot+sashiko
@ 2026-10-07  4:52   ` Eric Dumazet
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-10-07  4:52 UTC (permalink / raw)
  To: netdev-bot+sashiko, Joe Damato
  Cc: davem, kuba, pabeni, horms, netdev, stable, sfle, edumazet,
	michael.chan, pavan.chebbi, andrew+netdev, berni

Le mer. 7 oct. 2026 à 06:22, <netdev-bot+sashiko@kernel.org> a écrit :
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 (1) · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] Regression in the software UDP GSO path.

Ack, I CC Joe, because UDP GSO is not padding yet short UDP packets
(eg IPv4 and gso_size == 1)

I will send a V3, hopefully this will be the final one.

pw-bot: cr

>
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but the sister
>   bnge driver has the same stale-fragment ordering bug that the bnxt v2…

bnge driver will be fixed separately, once bnxt has shown the way.

>
> --
>
> bnxt_en: fix DMA mapping length for padded small packets
>
> This patch moves short-frame padding in bnxt_start_xmit() to the top of
> the function and uses skb_put_padto(). As a result, skb_headlen() now
> includes the pad when the linear head is DMA-mapped. This fixes the IOMMU
> read faults that happened when the TX BD length was larger than the
> mapped length.
>
> Closes: https://lore.kernel.org/netdev/20261004122616.56714cbd@nargothrond/
>
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > index d7728d0c5b6e6..7f379f21fe465 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>
> [ ... ]
>
> > @@ -507,6 +507,11 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
> >       txr = &bp->tx_ring[bp->tx_ring_map[i]];
> >       prod = txr->tx_prod;
> >
> > +     if (skb_put_padto(skb, BNXT_MIN_PKT_SIZE)) {
> > +             /* SKB already freed. */
> > +             goto tx_kick_pending;
> > +     }
> > +
>
> [Severity: Medium]
> Does moving the pad ahead of the SW USO branch change what
> bnxt_sw_udp_gso_xmit() puts on the wire?
>
> Before this patch, padding only ran at normal_tx:, after the SW USO branch
> had already returned. So bnxt_sw_udp_gso_xmit() always got the unpadded
> skb.
>
> Now a UDP GSO skb shorter than 52 bytes is padded first. On devices
> without BNXT_FLAG_UDP_GSO_CAP it then goes here:
>
>         if (skb_is_gso(skb) &&
>             (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) &&
>             !(bp->flags & BNXT_FLAG_UDP_GSO_CAP)) {
>                 int rc = bnxt_sw_udp_gso_xmit(bp, txr, txq, skb);
>
> bnxt_sw_udp_gso_xmit() in bnxt_gso.c then computes:
>
>         total_payload = skb->len - hdr_len;
>         num_segs = DIV_ROUND_UP(total_payload, mss);
>
> This counts the zero pad as UDP payload. tso_build_hdr() builds
> iph->tot_len and the UDP length from the segment payload, and the NIC
> fills in valid checksums. Would the receiver then accept the pad bytes as
> real UDP data?
>
> For example, Ethernet 14 + IPv4 20 + UDP 8 + 2 bytes of data is 44 bytes.
> With gso_size=1 that is padded to 52. total_payload goes from 2 to 10, so
> 10 datagrams are sent instead of 2, and eight of them carry only zeros.
>
> An unprivileged UDP socket can trigger this by setting UDP_SEGMENT to a
> small gso_size and sending a few bytes over IPv4.
>
> Also, BNXT_MIN_PKT_SIZE is a per-frame minimum. Padding the super-packet
> still leaves the individual SW USO segments (for example 43 bytes)
> shorter than that.
>
> Should the pad be skipped for skb_is_gso() skbs, or applied after the SW
> USO branch?
>
> [Severity: High]
> This isn't a bug introduced by this patch, but the sister bnge driver
> seems to have the ordering problem that this v2 placement avoids for
> bnxt. In bnge_start_xmit() in drivers/net/ethernet/broadcom/bnge/bnge_txrx.c,
> the frag count is read before padding:
>
>         last_frag = skb_shinfo(skb)->nr_frags;
>         ...
>         tx_buf->nr_frags = last_frag;
>         ...
>         if (eth_skb_pad(skb))
>                 goto tx_kick_pending;
>
> A nonlinear skb shorter than ETH_ZLEN has skb_tailroom() of 0. In that
> case __skb_pad() goes through pskb_expand_head() and skb_linearize().
> That pulls the frag data into the head, drops the frag page references
> and sets nr_frags to 0.
>
> bnge_start_xmit() then keeps using the stale last_frag:
>
>         flags = (len << TX_BD_LEN_SHIFT) | TX_BD_TYPE_LONG_TX_BD |
>                 TX_BD_CNT(last_frag + 2);
>         ...
>         for (i = 0; i < last_frag; i++) {
>                 frag = &skb_shinfo(skb)->frags[i];
>                 ...
>                 mapping = skb_frag_dma_map(bd->dev, frag, 0, len,
>                                            DMA_TO_DEVICE);
>
> Can this DMA-map frag pages that the skb no longer holds a reference to?
> If so, the NIC would send the frag data twice and read pages that may
> already be freed or recycled, such as page_pool pages.
>
> Two ways to get such skbs are AF_PACKET TX_RING SOCK_RAW frames and
> forwarded small packets whose payload stayed in a frag after
> pskb_trim_rcsum().
>
> As a separate fix, would it make sense to call eth_skb_pad() before
> nr_frags is read in bnge_start_xmit(), the same way this patch does for
> bnxt?
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006042153.199444-1-edumazet%40kernel.org

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-07  4:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-06  4:21 [PATCH v2 net] bnxt_en: fix DMA mapping length for padded small packets Eric Dumazet
2026-10-06  7:38 ` Michael Chan
2026-10-07  4:22 ` netdev-bot+sashiko
2026-10-07  4:52   ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox