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

[Lorenzo Bianconi]

> 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@xxxxxxxxxxxxx>
> 
>

Attachment: signature.asc
Description: PGP signature