From: Oleksij Rempel o.rempel@pengutronix.de
[ Upstream commit ee0aace5f844ef59335148875d05bec8764e71e8 ]
The stmmac_rx function would previously set skb->ip_summed to CHECKSUM_UNNECESSARY if hardware checksum offload (CoE) was enabled and the packet was of a known IP ethertype.
However, this logic failed to check if the hardware had actually reported a checksum error. The hardware status, indicating a header or payload checksum failure, was being ignored at this stage. This could cause corrupt packets to be passed up the network stack as valid.
This patch corrects the logic by checking the `csum_none` status flag, which is set when the hardware reports a checksum error. If this flag is set, skb->ip_summed is now correctly set to CHECKSUM_NONE, ensuring the kernel's network stack will perform its own validation and properly handle the corrupt packet.
Signed-off-by: Oleksij Rempel o.rempel@pengutronix.de Link: https://patch.msgid.link/20250818090217.2789521-2-o.rempel@pengutronix.de Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Explanation
- Bug fixed and user-visible - Current code marks all IP packets as hardware-verified when Rx checksum offload is enabled, even if hardware flagged a checksum error. See drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5738-5741: it sets `skb->ip_summed = CHECKSUM_UNNECESSARY` whenever `coe` is enabled and the packet has an IP ethertype, without considering hardware error status. - With enhanced descriptors, the hardware reports header or payload checksum failures via the `csum_none` status. The driver currently ignores this and can pass corrupted packets up the stack as if checksum was valid.
- What the patch changes - The patch adds the hardware error check to the decision: if `status & csum_none` is set, the driver does not mark the checksum as verified. Concretely, it changes the condition to - from: `if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb)) ... else skb->ip_summed = CHECKSUM_UNNECESSARY;` - to: `if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb) || (status & csum_none)) ... else skb->ip_summed = CHECKSUM_UNNECESSARY;` - This ensures `skb->ip_summed` remains `CHECKSUM_NONE` (asserted by `skb_checksum_none_assert(skb)`), so the network stack will compute/verify checksums in software and properly drop/handle corrupted packets.
- Why this is correct - For enhanced descriptors, the driver maps hardware status combinations indicating IP header or payload checksum errors to `csum_none` (i.e., “checksum not good”). See drivers/net/ethernet/stmicro/stmmac/enh_desc.c:105, 107, 109 where `enh_desc_coe_rdes0()` returns `csum_none` when the hardware indicates header/payload checksum errors. - The `csum_none` bit is explicitly defined as an Rx frame status in drivers/net/ethernet/stmicro/stmmac/common.h:343 (`enum rx_frame_status { ... csum_none = 0x2, ... }`). - Normal descriptor paths already drop errored frames early (e.g., see drivers/net/ethernet/stmicro/stmmac/norm_desc.c:52-100), so this change primarily corrects behavior for enhanced descriptors.
- Scope and risk - The change is small, localized to a single if-condition in `stmmac_rx()`. No architectural changes, no ABI changes. - If `csum_none` is set, the fix only downgrades to software verification, which is conservative and safe. The worst-case impact is mild extra CPU work for packets with checksum errors, which is acceptable. - For hardware/paths that never set `csum_none` (e.g., normal descriptors), behavior is unchanged.
- Security and correctness impact - Previously, packets with failed L3/L4 checksum could be marked as checksum-validated and accepted by upper layers. This is a correctness bug with potential security implications (transport- layer checksum bypass). The patch prevents that by forcing software verification when hardware signals errors.
- Additional note for completeness - There is a similar unconditional checksum-trust in the zero-copy/XDP dispatch path: drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5221-5224. It uses the same pattern and likely needs the same `(status & csum_none)` check. While not a blocker for this fix, stable trees should consider pulling a companion patch for the ZC path to ensure consistent behavior across receive paths.
- Stable backport criteria - Important bugfix affecting data integrity/correctness. - Minimal, contained change in a driver subsystem. - No new features, no architectural change, low regression risk. - Clear positive safety/security implications.
Given the above, this commit is a strong candidate for backporting to stable trees.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index b9f55e4e360fb..7a375de2258c4 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -5735,7 +5735,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
skb->protocol = eth_type_trans(skb, priv->dev);
- if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb)) + if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb) || + (status & csum_none)) skb_checksum_none_assert(skb); else skb->ip_summed = CHECKSUM_UNNECESSARY;