Re: [PATCH 01/13] 8139cp: fix DMA mapping unwind on TX frag map failure

[luyun <luyun_611@163.com>]

Hi,

A gentle ping for this series, the number of patches is 2, not 13. I made a mistake
when generating the patches. Please ignore it.

Any comments or reviews would be greatly appreciated.

Thanks,
Yun

在 2026/7/17 18:37, Yun Lu 写道:
> From: Yun Lu <luyun@kylinos.cn>
>
> When dma_map_single() fails for one of the TX fragments in
> cp_start_xmit(), unwind_tx_frag_mapping() is supposed to drop the
> DMA mappings that have been established so far. The current code
> gets this wrong in three ways:
>
> - it unmaps the DMA address read from the descriptor of the head
>    slot, but the head descriptor is only written after the fragment
>    loop, so a stale address of a previously completed (and already
>    unmapped) packet gets unmapped again;
> - the unmap sizes are off by one slot: slot first_entry + 1 + k
>    holds frags[k], but is unmapped with the size of frags[k + 1];
> - the head buffer mapping (first_mapping) is never unmapped at all,
>    leaking one DMA mapping on every failure.
>
> Pass the head mapping and its length to the unwind helper, unmap it
> explicitly, and unmap the successfully mapped fragments with their
> matching sizes.
>
> Fixes: cf3c4c03060b ("8139cp: Add dma_mapping_error checking")
> Signed-off-by: Yun Lu <luyun@kylinos.cn>
> ---
>   drivers/net/ethernet/realtek/8139cp.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
> index 5652da8a178c..03929bb0bd79 100644
> --- a/drivers/net/ethernet/realtek/8139cp.c
> +++ b/drivers/net/ethernet/realtek/8139cp.c
> @@ -713,13 +713,24 @@ static inline u32 cp_tx_vlan_tag(struct sk_buff *skb)
>   }
>   
>   static void unwind_tx_frag_mapping(struct cp_private *cp, struct sk_buff *skb,
> +				   dma_addr_t first_mapping, u32 first_len,
>   				   int first, int entry_last)
>   {
>   	int frag, index;
> -	struct cp_desc *txd;
> -	skb_frag_t *this_frag;
> -	for (frag = 0; frag+first < entry_last; frag++) {
> -		index = first+frag;
> +
> +	/* The head mapping was never written to the ring, unmap it
> +	 * using the saved address.
> +	 */
> +	dma_unmap_single(&cp->pdev->dev, first_mapping, first_len,
> +			 DMA_TO_DEVICE);
> +	cp->tx_skb[first] = NULL;
> +
> +	/* Unmap the frags that were successfully mapped. */
> +	for (frag = 0; first + 1 + frag < entry_last; frag++) {
> +		struct cp_desc *txd;
> +		skb_frag_t *this_frag;
> +
> +		index = first + 1 + frag;
>   		cp->tx_skb[index] = NULL;
>   		txd = &cp->tx_ring[index];
>   		this_frag = &skb_shinfo(skb)->frags[frag];
> @@ -828,7 +839,9 @@ static netdev_tx_t cp_start_xmit (struct sk_buff *skb,
>   						 skb_frag_address(this_frag),
>   						 len, DMA_TO_DEVICE);
>   			if (dma_mapping_error(&cp->pdev->dev, mapping)) {
> -				unwind_tx_frag_mapping(cp, skb, first_entry, entry);
> +				unwind_tx_frag_mapping(cp, skb, first_mapping,
> +						       first_len, first_entry,
> +						       entry);
>   				goto out_dma_error;
>   			}
>