[PATCH net] net: stmmac: fix stale descriptors and DMA mapping leak on Tx map failure

[PATCH net] net: stmmac: fix stale descriptors and DMA mapping leak on Tx map failure

[ZhaoJinming]

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.

Fixes: 30d932279dc2 ("net: stmmac: Add support for VLAN Insertion Offload")
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 29 +++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 24656b35350b14454fb10deced6516eb89e2c0c9..2e36c27e2cfb436af3566cf1c3e70d32ce9830a0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4769,12 +4769,12 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	unsigned int nopaged_len = skb_headlen(skb);
 	u32 queue = skb_get_queue_mapping(skb);
 	int nfrags = skb_shinfo(skb)->nr_frags;
-	unsigned int first_entry, tx_packets;
+	unsigned int first_entry, 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;
+	int first_tx, ret;
 	dma_addr_t dma_addr;
 	u32 sdu_len;
 
@@ -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))
 			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 <zhaojinming@uniontech.com>

Re: [PATCH net] net: stmmac: fix stale descriptors and DMA mapping leak on Tx map failure

[Maxime Chevallier]

Hi,

On 9/10/26 07:19, ZhaoJinming wrote:
> 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.
> 
> Fixes: 30d932279dc2 ("net: stmmac: Add support for VLAN Insertion Offload")
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 29 +++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 24656b35350b14454fb10deced6516eb89e2c0c9..2e36c27e2cfb436af3566cf1c3e70d32ce9830a0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4769,12 +4769,12 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  	unsigned int nopaged_len = skb_headlen(skb);
>  	u32 queue = skb_get_queue_mapping(skb);
>  	int nfrags = skb_shinfo(skb)->nr_frags;
> -	unsigned int first_entry, tx_packets;
> +	unsigned int first_entry, 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;
> +	int first_tx, ret;
>  	dma_addr_t dma_addr;
>  	u32 sdu_len;

Please follow the reverse xmas tree ordering, from longest line to shortest

The rest seems OK. By any chance, do you have a reproducer ?

Maxime

Re: [PATCH net] net: stmmac: fix stale descriptors and DMA mapping leak on Tx map failure

[Lorenzo Bianconi]

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

> In stmmac_xmit(), when the DMA mapping of the linear part or of a

[...]

>  	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 jumbo_frm() returns an error on the subsequent frames, entry is not updated
here, so we will end up with a DMA leak. Am I missing something?


> +		if (unlikely(ret < 0) && (ret != -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;

do we really need to update tx_q->cur_tx here?

Regards,
Lorenzo

> +	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 <zhaojinming@uniontech.com>
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net] net: stmmac: fix stale descriptors and DMA mapping leak on Tx map failure

[赵金明]

>> In stmmac_xmit(), when the DMA mapping of the linear part or of a



>



>[...]



>



>>? 	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 jumbo_frm() returns an error on the subsequent frames, entry is not updated



>here, so we will end up with a DMA leak. Am I missing something?



>
You're right, that was a real leak in v1. If jumbo_frm() failed while mapping a subsequent jumbo buffer, the buffers mapped before the failure were never unmapped, because jumbo_frm() returns -1 without reporting how many buffers it had already mapped, so the error path in stmmac_xmit() could not release them.

v2 fixes this inside jumbo_frm() itself: it now saves the starting entry and, when a subsequent dma_map_single() fails, unmaps the buffers it has already mapped before returning an error, in both ring and chain modes. This keeps the stmmac_xmit() error path unchanged and avoids changing jumbo_frm()'s return semantics.



>



>> +		if (unlikely(ret < 0) && (ret != -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;



>

>do we really need to update tx_q->cur_tx here?

Yes, it is needed. Two helpers advance tx_q->cur_tx before we reach the error path: stmmac_vlan_insert() (when a VLAN tag is present) moves it past the context descriptor, and stmmac_jumbo_frm() moves it past the jumbo head descriptors on success.

If we do not roll it back to first_tx, the released VLAN context descriptor (and any jumbo head descriptors) would be left inside the [dirty_tx, cur_tx) in-flight window, so stmmac_tx_clean() would treat them as completed frames, and the next xmit would skip those slots.

Unlike the TSO path, where the allocator uses a local entry and tx_q->cur_tx is only assigned on success, these are shared helpers that advance cur_tx as a side effect, so rolling it back here is the minimal correct fix.



>



>Regards,



>Lorenzo



>



>> +	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 <zhaojinming@uniontech.com>



>> 



>> 

Re: [PATCH net] net: stmmac: fix stale descriptors and DMA mapping leak on Tx map failure

[赵金明]

>Hi,



>



>On 9/10/26 07:19, ZhaoJinming wrote:



>> 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.



>> 



>> Fixes: 30d932279dc2 ("net: stmmac: Add support for VLAN Insertion Offload")



>> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>



>> ---



>>? drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 29 +++++++++++++++++++----



>>? 1 file changed, 25 insertions(+), 4 deletions(-)



>> 



>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c



>> index 24656b35350b14454fb10deced6516eb89e2c0c9..2e36c27e2cfb436af3566cf1c3e70d32ce9830a0 100644



>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c



>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c



>> @@ -4769,12 +4769,12 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)



>>? 	unsigned int nopaged_len = skb_headlen(skb);



>>? 	u32 queue = skb_get_queue_mapping(skb);



>>? 	int nfrags = skb_shinfo(skb)->nr_frags;



>> -	unsigned int first_entry, tx_packets;



>> +	unsigned int first_entry, 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;



>> +	int first_tx, ret;



>>? 	dma_addr_t dma_addr;



>>? 	u32 sdu_len;



>



>Please follow the reverse xmas tree ordering, from longest line to shortest

Done in v2: the variable declarations in stmmac_xmit() are now ordered
from longest to shortest line.



>



>The rest seems OK. By any chance, do you have a reproducer ?



>



>Maxime



>



>

No runtime reproducer, sorry. This was found by code review / static
analysis. The trigger requires a DMA mapping failure (dma_map_single /
skb_frag_dma_map) combined with VLAN insertion offload or fragmented
SKBs, which needs IOMMU pressure or fault injection to reproduce
reliably.

Thanks for the review.

Regards,
ZhaoJinming