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

[赵金明 <zhaojinming@uniontech.com>]

>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