[PATCH net 0/2] 8139cp: fix two TX error handling bugs

[PATCH net 0/2] 8139cp: fix two TX error handling bugs

[Yun Lu]

From: Yun Lu <luyun@kylinos.cn>

Hi,

This short series fixes two bugs in the transmit error handling of
the 8139cp driver. Both are only reachable when DMA mapping or
memory allocation fails, but each can take the kernel down when
triggered.

The first patch rewrites the DMA mapping unwind in cp_start_xmit().
When mapping a TX fragment fails, the current helper unmaps the
stale address found in the not-yet-written head descriptor slot,
which is a double unmap of a previously completed mapping; the
fragment unmaps use sizes shifted by one slot; and the head buffer
mapping is never unmapped at all, leaking one DMA mapping per
failure.

The second patch makes cp_tx_timeout() handle cp_init_rings()
failure. If the RX buffer refill fails there, the ring is left
empty, but the driver restarts the hardware and schedules NAPI
anyway, so cp_rx_poll() dereferences a NULL rx_skb slot and hits
BUG_ON(!skb). Bail out and leave the device down until the next
open instead.

Thanks,
Yun

Yun Lu (2):
  8139cp: fix DMA mapping unwind on TX frag map failure
  8139cp: handle cp_init_rings() failure in cp_tx_timeout()

 drivers/net/ethernet/realtek/8139cp.c | 31 +++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

-- 
2.25.1

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

[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;
 			}
 
-- 
2.25.1

[PATCH 02/13] 8139cp: handle cp_init_rings() failure in cp_tx_timeout()

[Yun Lu]

From: Yun Lu <luyun@kylinos.cn>

cp_tx_timeout() ignores the return value of cp_init_rings(). If the
RX buffer refill fails there, cp_clean_rings() has already dropped
all rx_skb entries, yet the driver restarts the hardware and
schedules NAPI unconditionally. cp_rx_poll() then dereferences a
NULL rx_skb slot and hits BUG_ON(!skb), crashing the kernel; the RX
ring also stays empty until the next close/open cycle.

On refill failure, keep the queue stopped and leave the device down
until the next open instead of restarting into a guaranteed BUG.

Fixes: 9030c0d24dbb ("8139cp: implement the missing dev->tx_timeout")
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
 drivers/net/ethernet/realtek/8139cp.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 03929bb0bd79..968bc0a2086e 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -1273,7 +1273,13 @@ static void cp_tx_timeout(struct net_device *dev, unsigned int txqueue)
 
 	cp_stop_hw(cp);
 	cp_clean_rings(cp);
-	cp_init_rings(cp);
+	if (cp_init_rings(cp)) {
+		netif_err(cp, tx_err, dev,
+			  "Failed to allocate RX buffers, not restarting\n");
+		netif_stop_queue(dev);
+		spin_unlock_irqrestore(&cp->lock, flags);
+		return;
+	}
 	cp_start_hw(cp);
 	__cp_set_rx_mode(dev);
 	cpw16_f(IntrMask, cp_norx_intr_mask);
-- 
2.25.1

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

[luyun]

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;
>   			}
>   

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

[Jakub Kicinski]

On Wed, 22 Jul 2026 15:37:02 +0800 luyun wrote:
> 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.

You must describe how your discovered the issue and what HW you have
tested it on. We don't take LLM fixes to Orphaned drivers.

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

[luyun]

在 2026/7/23 03:43, Jakub Kicinski 写道:
> On Wed, 22 Jul 2026 15:37:02 +0800 luyun wrote:
>> 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.
> You must describe how your discovered the issue and what HW you have
> tested it on. We don't take LLM fixes to Orphaned drivers.
Hi Jakub,

Thank you for taking the time to look at this, and sorry about the
patch numbering mess in my first submission.

On how the issue was found: it came out of a systematic audit of
the driver's error handling paths. I should be transparent that the
initial scan was assisted by AI tooling, but I then verified every
finding myself line by line before writing any patch.

Both bugs are deterministic logic errors provable by inspection.
Patch 1: in cp_start_xmit() the head descriptor is written only
after the fragment loop, but on a fragment mapping failure
unwind_tx_frag_mapping() unmaps the stale address still sitting in
that slot (a double unmap), uses fragment sizes shifted by one
slot, and never unmaps the head buffer mapping at all.
Patch 2: when cp_init_rings() fails, cp_clean_rings() has already
dropped all rx_skb entries, yet the hardware is restarted and NAPI
is scheduled unconditionally, so cp_rx_poll() hits BUG_ON(!skb).

On testing: the series is compile-tested only (full kbuild, -Werror
clean). I don't have RTL8139C+/RTL810x hardware, and QEMU's rtl8139
model is driven by 8139too, so I have no way to runtime-test this
driver. I want to be upfront about that.

I understand and respect the policy on orphaned drivers. I take
full responsibility for these changes, but I'd rather ask than assume:
are statically provable fixes like these acceptable for an orphaned
driver without hardware testing, or should I look for someone with
the hardware to confirm them first?

Thanks,
Yun