> In stmmac_xmit(), when the DMA mapping of the linear part or of a > fragment fails, the error path only frees the skb. This leaves behind > the DMA mappings already created for the linear part and for the > fragments mapped before the failure, which are never unmapped. > > The VLAN context descriptor programmed by stmmac_vlan_insert() is also > left behind with its OWN bit set while tx_q->cur_tx has been advanced > past it, so the DMA engine later consumes the orphaned descriptor and > applies its stale VLAN tag to an unrelated frame. > > Release the descriptors and their DMA mappings in the dma_map_err path > with stmmac_release_tx_desc() and stmmac_free_tx_buffer(), walking from > first_entry to entry, then roll back tx_q->cur_tx and release the VLAN > context descriptor. > > jumbo_frm() has the same issue: when the DMA mapping of a subsequent > jumbo buffer fails, the buffers already mapped are never unmapped. > Unmap them before returning an error in both ring and chain modes. > > Fixes: 30d932279dc2 ("net: stmmac: Add support for VLAN Insertion Offload") > Signed-off-by: ZhaoJinming Hi ZhaoJinming, some comments inline. Regards, Lorenzo > --- > Changes in v2: > - Reorder stmmac_xmit() variable declarations in reverse xmas tree > order. > - Unmap partially-mapped buffers in jumbo_frm() when a subsequent > buffer mapping fails (ring and chain modes). > - Link to v1: https://lore.kernel.org/r/20260910-stmmac-fix-vlan-desc-leak-v1-1-6b07dca5e5e8@uniontech.com > --- > drivers/net/ethernet/stmicro/stmmac/chain_mode.c | 19 +++++++++++++-- > drivers/net/ethernet/stmicro/stmmac/ring_mode.c | 10 +++++++- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 29 +++++++++++++++++++---- > 3 files changed, 51 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c > index 66025e2509e91f863571041f5b148d04e3708832..8415c740ee9eee32cd8b4c3e9b61517690b58f9d 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c > +++ b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c > @@ -20,6 +20,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb, > unsigned int nopaged_len = skb_headlen(skb); > struct stmmac_priv *priv = tx_q->priv_data; > unsigned int entry = tx_q->cur_tx; > + unsigned int first_entry = entry; > unsigned int bmax, buf_len; > unsigned int i = 1, len; > struct dma_desc *desc; > @@ -57,7 +58,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb, > bmax, DMA_TO_DEVICE); > desc->des2 = cpu_to_le32(lower_32_bits(des2)); > if (dma_mapping_error(priv->device, des2)) > - return -1; > + goto err_unmap; > tx_q->tx_skbuff_dma[entry].buf = des2; > tx_q->tx_skbuff_dma[entry].len = bmax; > stmmac_prepare_tx_desc(priv, desc, 0, bmax, csum, > @@ -70,7 +71,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb, > DMA_TO_DEVICE); > desc->des2 = cpu_to_le32(lower_32_bits(des2)); > if (dma_mapping_error(priv->device, des2)) > - return -1; > + goto err_unmap; > tx_q->tx_skbuff_dma[entry].buf = des2; > tx_q->tx_skbuff_dma[entry].len = len; > /* last descriptor can be set now */ > @@ -83,6 +84,20 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb, > tx_q->cur_tx = entry; > > return entry; > + > +err_unmap: > + while (first_entry != entry) { > + dma_unmap_single(priv->device, > + tx_q->tx_skbuff_dma[first_entry].buf, > + tx_q->tx_skbuff_dma[first_entry].len, > + DMA_TO_DEVICE); > + tx_q->tx_skbuff_dma[first_entry].buf = 0; > + tx_q->tx_skbuff_dma[first_entry].len = 0; are you missing stmmac_release_tx_desc() here? > + first_entry = STMMAC_NEXT_ENTRY(first_entry, > + priv->dma_conf.dma_tx_size); > + } > + > + return -1; > } > > static bool is_jumbo_frm(unsigned int len, bool enh_desc) > diff --git a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c > index f7949419eb9fdca891a50ad0956f40e17f9b0967..4170c897bd1a8b5b757b576250e82e5407cfa325 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c > +++ b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c > @@ -37,6 +37,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb, > len = nopaged_len - bmax; > > if (nopaged_len > BUF_SIZE_8KiB) { > + unsigned int first_entry = entry; > > des2 = dma_map_single(priv->device, skb->data, bmax, > DMA_TO_DEVICE); > @@ -62,8 +63,15 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb, > des2 = dma_map_single(priv->device, skb->data + bmax, len, > DMA_TO_DEVICE); > desc->des2 = cpu_to_le32(lower_32_bits(des2)); > - if (dma_mapping_error(priv->device, des2)) > + if (dma_mapping_error(priv->device, des2)) { > + dma_unmap_single(priv->device, > + tx_q->tx_skbuff_dma[first_entry].buf, > + bmax, DMA_TO_DEVICE); are you missing stmmac_release_tx_desc() here? > + tx_q->tx_skbuff_dma[first_entry].buf = 0; > + tx_q->tx_skbuff_dma[first_entry].len = 0; > + tx_q->tx_skbuff_dma[first_entry].is_jumbo = false; > return -1; > + } > tx_q->tx_skbuff_dma[entry].buf = des2; > tx_q->tx_skbuff_dma[entry].len = len; > tx_q->tx_skbuff_dma[entry].is_jumbo = true; > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 24656b35350b14454fb10deced6516eb89e2c0c9..9050611862108065dfa19c1a80961bd78073df9a 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -4767,15 +4767,15 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) > bool enh_desc, has_vlan, set_ic, is_jumbo = false; > struct stmmac_priv *priv = netdev_priv(dev); > unsigned int nopaged_len = skb_headlen(skb); > + unsigned int first_entry, entry, tx_packets; > u32 queue = skb_get_queue_mapping(skb); > int nfrags = skb_shinfo(skb)->nr_frags; > - unsigned int first_entry, tx_packets; > struct stmmac_txq_stats *txq_stats; > struct dma_desc *desc, *first_desc; > struct stmmac_tx_queue *tx_q; > int i, csum_insertion = 0; > - int entry, first_tx; > dma_addr_t dma_addr; > + int first_tx, ret; > u32 sdu_len; > > if (priv->tx_path_in_lpi_mode && priv->eee_sw_timer_en) > @@ -4832,9 +4832,10 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) > csum_insertion = skb->ip_summed == CHECKSUM_PARTIAL; > > if (unlikely(is_jumbo)) { > - entry = stmmac_jumbo_frm(priv, tx_q, skb, csum_insertion); > - if (unlikely(entry < 0) && (entry != -EINVAL)) > + ret = stmmac_jumbo_frm(priv, tx_q, skb, csum_insertion); > + if (unlikely(ret < 0) && (ret != -EINVAL)) can ret be really -EINVAL ? > goto dma_map_err; > + entry = ret; > } else { > bool last_segment = (nfrags == 0); > > @@ -4984,6 +4985,26 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) > > dma_map_err: > netdev_err(priv->dev, "Tx DMA map failed\n"); > + > + /* entry points one past the last descriptor written for this frame: > + * on failure it is the descriptor whose DMA mapping failed, so walk > + * from first_entry up to, but not including, entry. Reset cur_tx > + * unconditionally as both stmmac_vlan_insert() and stmmac_jumbo_frm() > + * may have advanced it, and release the VLAN context descriptor. > + */ > + while (first_entry != entry) { > + desc = stmmac_get_tx_desc(priv, tx_q, first_entry); > + stmmac_release_tx_desc(priv, desc, priv->descriptor_mode); > + stmmac_free_tx_buffer(priv, &priv->dma_conf, queue, first_entry); > + first_entry = STMMAC_NEXT_ENTRY(first_entry, > + priv->dma_conf.dma_tx_size); > + } > + > + tx_q->cur_tx = first_tx; > + if (has_vlan) { > + desc = stmmac_get_tx_desc(priv, tx_q, first_tx); > + stmmac_release_tx_desc(priv, desc, priv->descriptor_mode); > + } > max_sdu_err: > dev_kfree_skb(skb); > priv->xstats.tx_dropped++; > > --- > base-commit: 893e11787f78e43b534e252249ac3fff4d1333f8 > change-id: 20260909-stmmac-fix-vlan-desc-leak-f057bb061daa > > Best regards, > -- > ZhaoJinming > >