Skip to contentSkip to conversation

hinic3: Fix skb linearization mismatch and drop skb when skb_checksum_help() failed

PATCH Applied Fan Gong 3 participants 3 messages Aug 11 – Aug 17

Subsystems (4)
lore
Share & tools
lore

Fan Gong

author maintainer
#0 lore raw

PATCH v4 1/1 hinic3: Fix skb linearization mismatch and drop skb when skb_checksum_help() failed

Previously, hinic3_send_one_skb() cached the skb fragment count before calling hinic3_tx_offload(). If hinic3_tx_csum() falls back to skb_checksum_help() for unsupported tunnel packets, the skb may be linearized. Continuing to build the TX descriptor with the stale fragment count leads to a descriptor mismatch, which can trigger out-of-bounds DMA reads or IOMMU faults.

Furthermore, the old code ignored the return value of skb_checksum_help(), transmitting corrupted packets with incomplete checksums upon failure.

Fix this by:
1. Moving the hinic3_tx_offload() call before calculating 'num_sge' to ensure the correct fragment count is used if the SKB is linearized.
2. Propagating skb_checksum_help() errors and returning HINIC3_TX_OFFLOAD_INVALID to properly drop the skb.

Fixes 17fcb3dc12bb ("hinic3: module initialization and tx/rx logic")
---
 drivers/net/ethernet/huawei/hinic3/hinic3_tx.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/net/ethernet/huawei/hinic3/hinic3_tx.c +6 -3
index 9306bf0020ca..cc541e7a2318 100644
@@ -261,8 +261,7 @@ static int hinic3_tx_csum(struct hinic3_txq *txq, struct hinic3_sq_task *task,
 		    ((struct udphdr *)skb_transport_header(skb))->dest !=
 		    VXLAN_OFFLOAD_PORT_LE) {
 			/* Unsupported tunnel packet, disable csum offload */
-			skb_checksum_help(skb);
-			return 0;
+			return skb_checksum_help(skb);
 		}
 	}
 
@@ -412,6 +411,10 @@ static u32 hinic3_tx_offload(struct sk_buff *skb, struct hinic3_sq_task *task,
 		offload |= HINIC3_TX_OFFLOAD_TSO;
 	} else {
 		tso_cs_en = hinic3_tx_csum(txq, task, skb);
+		if (tso_cs_en < 0) {
+			offload = HINIC3_TX_OFFLOAD_INVALID;
+			return offload;
+		}
 		if (tso_cs_en)
 			offload |= HINIC3_TX_OFFLOAD_CSUM;
 	}
@@ -545,6 +548,7 @@ static netdev_tx_t hinic3_send_one_skb(struct sk_buff *skb,
 		skb->len = MIN_SKB_LEN;
 	}
 
+	offload = hinic3_tx_offload(skb, &task, &queue_info, txq);
 	num_sge = skb_shinfo(skb)->nr_frags + 1;
 	/* assume normal wqe format + 1 wqebb for task info */
 	wqebb_cnt = num_sge + 1;
@@ -560,7 +564,6 @@ static netdev_tx_t hinic3_send_one_skb(struct sk_buff *skb,
 		return NETDEV_TX_BUSY;
 	}
 
-	offload = hinic3_tx_offload(skb, &task, &queue_info, txq);
 	if (unlikely(offload == HINIC3_TX_OFFLOAD_INVALID)) {
 		goto err_drop_pkt;
 	} else if (!offload) {

base-commit: 2195424c3da2ef1829a63b807e3a900a90e57d85 --
2.54.0

reviewer Reviewed-by on v4 1/1
#1 lore raw
Quoted 16 lines · Fan Gong

Previously, hinic3_send_one_skb() cached the skb fragment count before calling hinic3_tx_offload(). If hinic3_tx_csum() falls back to skb_checksum_help() for unsupported tunnel packets, the skb may be linearized. Continuing to build the TX descriptor with the stale fragment count leads to a descriptor mismatch, which can trigger out-of-bounds DMA reads or IOMMU faults.

Furthermore, the old code ignored the return value of skb_checksum_help(), transmitting corrupted packets with incomplete checksums upon failure.

Fix this by:
1. Moving the hinic3_tx_offload() call before calculating 'num_sge' to ensure the correct fragment count is used if the SKB is linearized.
2. Propagating skb_checksum_help() errors and returning HINIC3_TX_OFFLOAD_INVALID to properly drop the skb.

Quoted 6 lines
Fixes 17fcb3dc12bb ("hinic3: module initialization and tx/rx logic")

Thanks,

I do think this change is fine as it addresses the issue described.

But you may want to consider some follow-up based on the AI-generated review of v1 that was forwarded by Jakub:

https://lore.kernel.org/netdev/[email protected]/

...

maintainer Applied
#2 lore raw
In reply to Fan Gong
Quoted 6 lines · On Tue, 11 Aug 2026 19:43:59 +0800 Fan Gong wrote:

Previously, hinic3_send_one_skb() cached the skb fragment count before calling hinic3_tx_offload(). If hinic3_tx_csum() falls back to skb_checksum_help() for unsupported tunnel packets, the skb may be linearized. Continuing to build the TX descriptor with the stale fragment count leads to a descriptor mismatch, which can trigger out-of-bounds DMA reads or IOMMU faults.

Applied, thanks!

Signature · 1 line
pw-bot: accept