All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Woudstra <ericwouds@gmail.com>
To: Pablo Neira Ayuso <pablo@netfilter.org>,
	Florian Westphal <fw@strlen.de>, Phil Sutter <phil@nwl.cc>,
	Nikolay Aleksandrov <razor@blackwall.org>,
	Ido Schimmel <idosch@nvidia.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>
Cc: netfilter-devel@vger.kernel.org, bridge@lists.linux.dev,
	netdev@vger.kernel.org, Eric Woudstra <ericwouds@gmail.com>
Subject: [PATCH v20 nf-next 1/2] netfilter: utils: nf_ip(6)_checksum(_partial) correct data!=networkheader
Date: Tue, 12 May 2026 12:33:46 +0200	[thread overview]
Message-ID: <20260512103347.102746-2-ericwouds@gmail.com> (raw)
In-Reply-To: <20260512103347.102746-1-ericwouds@gmail.com>

In the conntrack hook it may not always be the case that:
skb_network_header(skb) == skb->data, i.e. skb_network_offset(skb)
is zero.

This is problematic when L4 function nf_conntrack_handle_packet(),
nf_conntrack_icmpv4/6_error() and other functions alike are accessing L3
data. These functions also calculate the checksum using
nf_ip(6)_checksum() and nf_ip(6)_checksum_partial().

They in turn use lower skb-checksum functions that are based on using
skb->data and will fail when skb_network_offset(skb) is not zero.

Adjust for skb_network_offset(skb), so that the checksum is calculated
correctly.

Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
---
 net/netfilter/utils.c | 52 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/utils.c b/net/netfilter/utils.c
index 29c4dcc362c74..b738444c9cb6f 100644
--- a/net/netfilter/utils.c
+++ b/net/netfilter/utils.c
@@ -10,9 +10,18 @@
 __sum16 nf_ip_checksum(struct sk_buff *skb, unsigned int hook,
 		       unsigned int dataoff, u8 protocol)
 {
+	unsigned int nhpull = skb_network_offset(skb);
 	const struct iphdr *iph = ip_hdr(skb);
 	__sum16 csum = 0;
 
+	if (WARN_ON_ONCE(!skb_pointer_if_linear(skb, nhpull, 0)))
+		return 0;
+
+	/* pull/push because the lower csum functions assume that
+	 * skb_network_offset(skb) is zero.
+	 */
+	dataoff -= nhpull;
+	__skb_pull(skb, nhpull);
 	switch (skb->ip_summed) {
 	case CHECKSUM_COMPLETE:
 		if (hook != NF_INET_PRE_ROUTING && hook != NF_INET_LOCAL_IN)
@@ -35,6 +44,7 @@ __sum16 nf_ip_checksum(struct sk_buff *skb, unsigned int hook,
 						       protocol, 0);
 		csum = __skb_checksum_complete(skb);
 	}
+	__skb_push(skb, nhpull);
 	return csum;
 }
 EXPORT_SYMBOL(nf_ip_checksum);
@@ -44,29 +54,47 @@ static __sum16 nf_ip_checksum_partial(struct sk_buff *skb, unsigned int hook,
 				      unsigned int dataoff, unsigned int len,
 				      u8 protocol)
 {
+	unsigned int nhpull = skb_network_offset(skb);
 	const struct iphdr *iph = ip_hdr(skb);
 	__sum16 csum = 0;
 
+	if (WARN_ON_ONCE(!skb_pointer_if_linear(skb, nhpull, 0)))
+		return 0;
+
+	/* See nf_ip_checksum() */
+	dataoff -= nhpull;
+	__skb_pull(skb, nhpull);
 	switch (skb->ip_summed) {
 	case CHECKSUM_COMPLETE:
-		if (len == skb->len - dataoff)
-			return nf_ip_checksum(skb, hook, dataoff, protocol);
+		if (len == skb->len - dataoff) {
+			csum = nf_ip_checksum(skb, hook, dataoff, protocol);
+			break;
+		}
 		fallthrough;
 	case CHECKSUM_NONE:
 		skb->csum = csum_tcpudp_nofold(iph->saddr, iph->daddr, protocol,
 					       skb->len - dataoff, 0);
 		skb->ip_summed = CHECKSUM_NONE;
-		return __skb_checksum_complete_head(skb, dataoff + len);
+		csum = __skb_checksum_complete_head(skb, dataoff + len);
+		break;
 	}
+	__skb_push(skb, nhpull);
 	return csum;
 }
 
 __sum16 nf_ip6_checksum(struct sk_buff *skb, unsigned int hook,
 			unsigned int dataoff, u8 protocol)
 {
+	unsigned int nhpull = skb_network_offset(skb);
 	const struct ipv6hdr *ip6h = ipv6_hdr(skb);
 	__sum16 csum = 0;
 
+	if (WARN_ON_ONCE(!skb_pointer_if_linear(skb, nhpull, 0)))
+		return 0;
+
+	/* See nf_ip_checksum() */
+	dataoff -= nhpull;
+	__skb_pull(skb, nhpull);
 	switch (skb->ip_summed) {
 	case CHECKSUM_COMPLETE:
 		if (hook != NF_INET_PRE_ROUTING && hook != NF_INET_LOCAL_IN)
@@ -89,7 +117,9 @@ __sum16 nf_ip6_checksum(struct sk_buff *skb, unsigned int hook,
 						      skb_checksum(skb, 0,
 								   dataoff, 0))));
 		csum = __skb_checksum_complete(skb);
+		break;
 	}
+	__skb_push(skb, nhpull);
 	return csum;
 }
 EXPORT_SYMBOL(nf_ip6_checksum);
@@ -98,14 +128,23 @@ static __sum16 nf_ip6_checksum_partial(struct sk_buff *skb, unsigned int hook,
 				       unsigned int dataoff, unsigned int len,
 				       u8 protocol)
 {
+	unsigned int nhpull = skb_network_offset(skb);
 	const struct ipv6hdr *ip6h = ipv6_hdr(skb);
 	__wsum hsum;
 	__sum16 csum = 0;
 
+	if (WARN_ON_ONCE(!skb_pointer_if_linear(skb, nhpull, 0)))
+		return 0;
+
+	/* See nf_ip_checksum() */
+	dataoff -= nhpull;
+	__skb_pull(skb, nhpull);
 	switch (skb->ip_summed) {
 	case CHECKSUM_COMPLETE:
-		if (len == skb->len - dataoff)
-			return nf_ip6_checksum(skb, hook, dataoff, protocol);
+		if (len == skb->len - dataoff) {
+			csum = nf_ip6_checksum(skb, hook, dataoff, protocol);
+			break;
+		}
 		fallthrough;
 	case CHECKSUM_NONE:
 		hsum = skb_checksum(skb, 0, dataoff, 0);
@@ -115,8 +154,9 @@ static __sum16 nf_ip6_checksum_partial(struct sk_buff *skb, unsigned int hook,
 							 protocol,
 							 csum_sub(0, hsum)));
 		skb->ip_summed = CHECKSUM_NONE;
-		return __skb_checksum_complete_head(skb, dataoff + len);
+		csum = __skb_checksum_complete_head(skb, dataoff + len);
 	}
+	__skb_push(skb, nhpull);
 	return csum;
 };
 
-- 
2.53.0


  reply	other threads:[~2026-05-12 10:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12 10:33 [PATCH v20 nf-next 0/2] conntrack: bridge: add double vlan, pppoe and pppoe-in-q Eric Woudstra
2026-05-12 10:33 ` Eric Woudstra [this message]
2026-05-12 10:33 ` [PATCH v20 nf-next 2/2] netfilter: bridge: Add conntrack double vlan and pppoe Eric Woudstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260512103347.102746-2-ericwouds@gmail.com \
    --to=ericwouds@gmail.com \
    --cc=bridge@lists.linux.dev \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pablo@netfilter.org \
    --cc=phil@nwl.cc \
    --cc=razor@blackwall.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.