mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] ipv4: validate checksum_start before completing checksum
@ 2026-10-07 22:42 Michael S. Tsirkin
  2026-10-07 23:36 ` Willem de Bruijn
  2026-10-08 18:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2026-10-07 22:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Pablo Neira Ayuso,
	Florian Westphal, Phil Sutter, Nikolay Aleksandrov,
	Steffen Klassert, Herbert Xu, Willem de Bruijn, netdev,
	netfilter-devel, coreteam, bridge

If a packet with bad checksum metadata gets into the ipv4 stack,
skb_checksum_help can corrupt the network header and cause a bunch of
mischief.

This was discovered and reported by Paulos, and has been reporoduced
by others independently since.

We really shouldn't allow such packets in, but as a defence
in depth measure, let's also check before we complete the checksum.

A more complete validation at input is forthcoming, but needs more work.

Assisted-by: LLM
Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
Reported-by: Paulos Yibelo <habte.yibelo@gmail.com>
Closes: https://lore.kernel.org/netdev/20260922030310.8684-2-habte.yibelo@gmail.com/
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Lightly tested.

 include/net/ip.h                           |  7 +++++++
 net/bridge/netfilter/nf_conntrack_bridge.c | 10 +++++++---
 net/ipv4/ip_output.c                       | 10 +++++++---
 net/netfilter/nfnetlink_queue.c            |  7 +++++++
 net/netfilter/xt_CHECKSUM.c                |  6 +++++-
 net/xfrm/xfrm_output.c                     | 16 +++++++++++-----
 6 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/include/net/ip.h b/include/net/ip.h
index 6f602df72ee6..d07c2c573024 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -74,6 +74,13 @@ static inline unsigned int ip_hdrlen(const struct sk_buff *skb)
 	return ip_hdr(skb)->ihl * 4;
 }
 
+static inline int ip_check_csum_start(const struct sk_buff *skb)
+{
+	if (unlikely(skb->csum_start < skb->network_header + ip_hdrlen(skb)))
+		return -EINVAL;
+	return 0;
+}
+
 struct ipcm_cookie {
 	struct sockcm_cookie	sockc;
 	__be32			addr;
diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c
index 7ecb8a26bfa3..b5444335b86f 100644
--- a/net/bridge/netfilter/nf_conntrack_bridge.c
+++ b/net/bridge/netfilter/nf_conntrack_bridge.c
@@ -39,9 +39,13 @@ static int nf_br_ip_fragment(struct net *net, struct sock *sk,
 	int err = 0;
 
 	/* for offloaded checksums cleanup checksum before fragmentation */
-	if (skb->ip_summed == CHECKSUM_PARTIAL &&
-	    (err = skb_checksum_help(skb)))
-		goto blackhole;
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		err = ip_check_csum_start(skb);
+		if (!err)
+			err = skb_checksum_help(skb);
+		if (err)
+			goto blackhole;
+	}
 
 	iph = ip_hdr(skb);
 
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 74e095b6b7ca..bea79611617f 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -771,9 +771,13 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
 	int err = 0;
 
 	/* for offloaded checksums cleanup checksum before fragmentation */
-	if (skb->ip_summed == CHECKSUM_PARTIAL &&
-	    (err = skb_checksum_help(skb)))
-		goto fail;
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		err = ip_check_csum_start(skb);
+		if (!err)
+			err = skb_checksum_help(skb);
+		if (err)
+			goto fail;
+	}
 
 	/*
 	 *	Point into the IP datagram header.
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index c727668b0c5b..5177e27174e7 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -40,6 +40,7 @@
 #include <linux/udp.h>
 #include <net/gre.h>
 #include <net/gso.h>
+#include <net/ip.h>
 #include <net/sock.h>
 #include <net/tcp_states.h>
 #include <net/netfilter/nf_queue.h>
@@ -674,6 +675,12 @@ static int nfqnl_put_bridge(struct nf_queue_entry *entry, struct sk_buff *skb)
 
 static int nf_queue_checksum_help(struct sk_buff *entskb)
 {
+	if (entskb->protocol == htons(ETH_P_IP)) {
+		if (!pskb_network_may_pull(entskb, sizeof(struct iphdr)) ||
+		    ip_check_csum_start(entskb))
+			return -EINVAL;
+	}
+
 	if (skb_csum_is_sctp(entskb))
 		return skb_crc32c_csum_help(entskb);
 
diff --git a/net/netfilter/xt_CHECKSUM.c b/net/netfilter/xt_CHECKSUM.c
index 9d99f5a3d176..1fb0f8118404 100644
--- a/net/netfilter/xt_CHECKSUM.c
+++ b/net/netfilter/xt_CHECKSUM.c
@@ -15,6 +15,7 @@
 
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv6/ip6_tables.h>
+#include <net/ip.h>
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Michael S. Tsirkin <mst@redhat.com>");
@@ -25,8 +26,11 @@ MODULE_ALIAS("ip6t_CHECKSUM");
 static unsigned int
 checksum_tg(struct sk_buff *skb, const struct xt_action_param *par)
 {
-	if (skb->ip_summed == CHECKSUM_PARTIAL && !skb_is_gso(skb))
+	if (skb->ip_summed == CHECKSUM_PARTIAL && !skb_is_gso(skb)) {
+		if (xt_family(par) == NFPROTO_IPV4 && ip_check_csum_start(skb))
+			return NF_DROP;
 		skb_checksum_help(skb);
+	}
 
 	return XT_CONTINUE;
 }
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index e305ba32e356..f1f612cd7b43 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -823,16 +823,22 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
 	}
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		err = skb_checksum_help(skb);
-		if (err) {
-			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
-			kfree_skb(skb);
-			return err;
+		if (skb->protocol == htons(ETH_P_IP)) {
+			err = ip_check_csum_start(skb);
+			if (err)
+				goto error;
 		}
+		err = skb_checksum_help(skb);
+		if (err)
+			goto error;
 	}
 
 out:
 	return xfrm_output2(net, sk, skb);
+error:
+	XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
+	kfree_skb(skb);
+	return err;
 }
 EXPORT_SYMBOL_GPL(xfrm_output);
 
-- 
MST


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] ipv4: validate checksum_start before completing checksum
  2026-10-07 22:42 [PATCH net] ipv4: validate checksum_start before completing checksum Michael S. Tsirkin
@ 2026-10-07 23:36 ` Willem de Bruijn
  2026-10-08 18:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Willem de Bruijn @ 2026-10-07 23:36 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Ahern, Ido Schimmel, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	Nikolay Aleksandrov, Steffen Klassert, Herbert Xu, netdev,
	netfilter-devel, coreteam, bridge

On Wed, Oct 7, 2026 at 6:42 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> If a packet with bad checksum metadata gets into the ipv4 stack,
> skb_checksum_help can corrupt the network header and cause a bunch of
> mischief.
>
> This was discovered and reported by Paulos, and has been reporoduced

(minor) typo: reproduced

> by others independently since.
>
> We really shouldn't allow such packets in, but as a defence
> in depth measure, let's also check before we complete the checksum.
>
> A more complete validation at input is forthcoming, but needs more work.
>
> Assisted-by: LLM
> Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
> Reported-by: Paulos Yibelo <habte.yibelo@gmail.com>
> Closes: https://lore.kernel.org/netdev/20260922030310.8684-2-habte.yibelo@gmail.com/
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Cc: stable@vger.kernel.org

Reviewed-by: Willem de Bruijn <willemb@google.com>

One point about sending as a single patch touching five separate
modules: nf_br_ip_fragment was only introduced in Linux 5.3.
Such a block is probably easily dropped from a stable backport.
A single patch is likely still preferable over splitting into two
(core + optional) or even five (one per module) patches.

> ---
>
> Lightly tested.

Tested by me as well and found to block the known bad input.

Touching five modules is not ideal, but could not find any other
approach that would be preferable: low risk of unintended
consequences, out of the hot path, and relatively concise.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] ipv4: validate checksum_start before completing checksum
  2026-10-07 22:42 [PATCH net] ipv4: validate checksum_start before completing checksum Michael S. Tsirkin
  2026-10-07 23:36 ` Willem de Bruijn
@ 2026-10-08 18:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-08 18:40 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, dsahern, idosch, davem, edumazet, kuba, pabeni,
	horms, pablo, fw, phil, razor, steffen.klassert, herbert,
	willemdebruijn.kernel, netdev, netfilter-devel, coreteam, bridge

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 7 Oct 2026 18:42:49 -0400 you wrote:
> If a packet with bad checksum metadata gets into the ipv4 stack,
> skb_checksum_help can corrupt the network header and cause a bunch of
> mischief.
> 
> This was discovered and reported by Paulos, and has been reporoduced
> by others independently since.
> 
> [...]

Here is the summary with links:
  - [net] ipv4: validate checksum_start before completing checksum
    https://git.kernel.org/netdev/net/c/6785011f8b16

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-08 18:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-07 22:42 [PATCH net] ipv4: validate checksum_start before completing checksum Michael S. Tsirkin
2026-10-07 23:36 ` Willem de Bruijn
2026-10-08 18:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®