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

[Maxime Chevallier <maxime.chevallier@bootlin.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

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

Maxime