From: Kyle Zeng <kylebot@openai.com>
To: dev@openvswitch.org
Cc: netdev@vger.kernel.org, aconole@redhat.com, echaudro@redhat.com,
i.maximets@ovn.org, davem@davemloft.net, kuba@kernel.org,
edumazet@kernel.org, pabeni@redhat.com, horms@kernel.org,
linux-kernel@vger.kernel.org, Kyle Zeng <kylebot@openai.com>,
stable@vger.kernel.org
Subject: [PATCH net v3] net: openvswitch: make transport checksums writable
Date: Fri, 13 Nov 2026 15:21:58 -0800 [thread overview]
Message-ID: <20261113232158.61174-1-kylebot@openai.com> (raw)
OVS can still rewrite an IP address after transport-header extraction
fails. The checksum helpers check skb->len, which includes non-linear
data, before directly accessing the L4 checksum. If the transport header
was not pulled into the linear head, the checksum update can read or
write beyond it.
Call skb_ensure_writable() before IPv4 TCP/UDP and IPv6 TCP/UDP/ICMPv6
checksum updates. Save the original addresses and reload header pointers,
since even a failed pull can move the skb head.
Keep failures nonfatal to preserve the existing policy of rewriting IP
addresses when the transport header is unavailable. A failed pull or
unshare may leave a stale checksum or CHECKSUM_PARTIAL seed; the receiver
may discard the packet.
The issue was found by an LLM.
Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")
Fixes: 3fdbd1ce11e5 ("openvswitch: add ipv6 'set' action")
Suggested-by: Ilya Maximets <i.maximets@ovn.org>
Cc: stable@vger.kernel.org
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
v3:
- Select source and destination addresses with booleans instead of
calculating offsets. Keep the saved IPv6 address across head movement.
- Remove the redundant IPv6 fragment check and correct tab alignment.
- Move testing details below the separator and clean up attribution.
v2: https://lore.kernel.org/r/20261107040238.27606-1-kylebot@openai.com
v1: https://lore.kernel.org/r/20261030042304.74291-1-kylebot@openai.com
I have a reproducer for this issue and can share through DM.
net/openvswitch/actions.c | 77 +++++++++++++++++++++++++--------------
1 file changed, 49 insertions(+), 28 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 12e339cbf791..3b10b3fbadc3 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -319,20 +319,22 @@ static int pop_nsh(struct sk_buff *skb, struct sw_flow_key *key)
return 0;
}
-static void update_ip_l4_checksum(struct sk_buff *skb, struct iphdr *nh,
- __be32 addr, __be32 new_addr)
+static void update_ip_l4_checksum(struct sk_buff *skb, __be32 addr,
+ __be32 new_addr)
{
- int transport_len = skb->len - skb_transport_offset(skb);
+ struct iphdr *nh = ip_hdr(skb);
if (nh->frag_off & htons(IP_OFFSET))
return;
if (nh->protocol == IPPROTO_TCP) {
- if (likely(transport_len >= sizeof(struct tcphdr)))
+ if (likely(!skb_ensure_writable(skb, skb_transport_offset(skb) +
+ sizeof(struct tcphdr))))
inet_proto_csum_replace4(&tcp_hdr(skb)->check, skb,
addr, new_addr, true);
} else if (nh->protocol == IPPROTO_UDP) {
- if (likely(transport_len >= sizeof(struct udphdr))) {
+ if (likely(!skb_ensure_writable(skb, skb_transport_offset(skb) +
+ sizeof(struct udphdr)))) {
struct udphdr *uh = udp_hdr(skb);
if (uh->check || skb->ip_summed == CHECKSUM_PARTIAL) {
@@ -345,32 +347,34 @@ static void update_ip_l4_checksum(struct sk_buff *skb, struct iphdr *nh,
}
}
-static void set_ip_addr(struct sk_buff *skb, struct iphdr *nh,
- __be32 *addr, __be32 new_addr)
+/* May move the skb head; callers must reload packet pointers. */
+static void set_ip_addr(struct sk_buff *skb, __be32 new_addr, bool saddr)
{
- update_ip_l4_checksum(skb, nh, *addr, new_addr);
- csum_replace4(&nh->check, *addr, new_addr);
+ __be32 old_addr = saddr ? ip_hdr(skb)->saddr : ip_hdr(skb)->daddr;
+
+ update_ip_l4_checksum(skb, old_addr, new_addr);
+ csum_replace4(&ip_hdr(skb)->check, old_addr, new_addr);
+
skb_clear_hash(skb);
ovs_ct_clear(skb, NULL);
- *addr = new_addr;
+
+ if (saddr)
+ ip_hdr(skb)->saddr = new_addr;
+ else
+ ip_hdr(skb)->daddr = new_addr;
}
static void update_ipv6_checksum(struct sk_buff *skb, u8 l4_proto,
__be32 addr[4], const __be32 new_addr[4])
{
- int transport_len;
-
- if (l4_proto == NEXTHDR_FRAGMENT)
- return;
-
- transport_len = skb->len - skb_transport_offset(skb);
-
if (l4_proto == NEXTHDR_TCP) {
- if (likely(transport_len >= sizeof(struct tcphdr)))
+ if (likely(!skb_ensure_writable(skb, skb_transport_offset(skb) +
+ sizeof(struct tcphdr))))
inet_proto_csum_replace16(&tcp_hdr(skb)->check, skb,
addr, new_addr, true);
} else if (l4_proto == NEXTHDR_UDP) {
- if (likely(transport_len >= sizeof(struct udphdr))) {
+ if (likely(!skb_ensure_writable(skb, skb_transport_offset(skb) +
+ sizeof(struct udphdr)))) {
struct udphdr *uh = udp_hdr(skb);
if (uh->check || skb->ip_summed == CHECKSUM_PARTIAL) {
@@ -381,7 +385,8 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 l4_proto,
}
}
} else if (l4_proto == NEXTHDR_ICMP) {
- if (likely(transport_len >= sizeof(struct icmp6hdr)))
+ if (likely(!skb_ensure_writable(skb, skb_transport_offset(skb) +
+ sizeof(struct icmp6hdr))))
inet_proto_csum_replace16(&icmp6_hdr(skb)->icmp6_cksum,
skb, addr, new_addr, true);
}
@@ -396,16 +401,28 @@ static void mask_ipv6_addr(const __be32 old[4], const __be32 addr[4],
masked[3] = OVS_MASKED(old[3], addr[3], mask[3]);
}
+/* May move the skb head; callers must reload packet pointers. */
static void set_ipv6_addr(struct sk_buff *skb, u8 l4_proto,
- __be32 addr[4], const __be32 new_addr[4],
+ const __be32 new_addr[4], bool saddr,
bool recalculate_csum)
{
- if (recalculate_csum)
- update_ipv6_checksum(skb, l4_proto, addr, new_addr);
+ struct ipv6hdr *nh = ipv6_hdr(skb);
+ __be32 old_addr[4];
+
+ if (recalculate_csum) {
+ memcpy(old_addr, saddr ? &nh->saddr : &nh->daddr,
+ sizeof(old_addr));
+ update_ipv6_checksum(skb, l4_proto, old_addr, new_addr);
+ nh = ipv6_hdr(skb);
+ }
skb_clear_hash(skb);
ovs_ct_clear(skb, NULL);
- memcpy(addr, new_addr, sizeof(__be32[4]));
+
+ if (saddr)
+ memcpy(&nh->saddr, new_addr, sizeof(__be32[4]));
+ else
+ memcpy(&nh->daddr, new_addr, sizeof(__be32[4]));
}
static void set_ipv6_dsfield(struct sk_buff *skb, struct ipv6hdr *nh, u8 ipv6_tclass, u8 mask)
@@ -479,16 +496,18 @@ static int set_ipv4(struct sk_buff *skb, struct sw_flow_key *flow_key,
new_addr = OVS_MASKED(nh->saddr, key->ipv4_src, mask->ipv4_src);
if (unlikely(new_addr != nh->saddr)) {
- set_ip_addr(skb, nh, &nh->saddr, new_addr);
+ set_ip_addr(skb, new_addr, true);
flow_key->ipv4.addr.src = new_addr;
+ nh = ip_hdr(skb);
}
}
if (mask->ipv4_dst) {
new_addr = OVS_MASKED(nh->daddr, key->ipv4_dst, mask->ipv4_dst);
if (unlikely(new_addr != nh->daddr)) {
- set_ip_addr(skb, nh, &nh->daddr, new_addr);
+ set_ip_addr(skb, new_addr, false);
flow_key->ipv4.addr.dst = new_addr;
+ nh = ip_hdr(skb);
}
}
if (mask->ipv4_tos) {
@@ -533,10 +552,11 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *flow_key,
mask_ipv6_addr(saddr, key->ipv6_src, mask->ipv6_src, masked);
if (unlikely(memcmp(saddr, masked, sizeof(masked)))) {
- set_ipv6_addr(skb, flow_key->ip.proto, saddr, masked,
+ set_ipv6_addr(skb, flow_key->ip.proto, masked, true,
true);
memcpy(&flow_key->ipv6.addr.src, masked,
sizeof(flow_key->ipv6.addr.src));
+ nh = ipv6_hdr(skb);
}
}
if (is_ipv6_mask_nonzero(mask->ipv6_dst)) {
@@ -555,10 +575,11 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *flow_key,
NULL, &flags)
!= NEXTHDR_ROUTING);
- set_ipv6_addr(skb, flow_key->ip.proto, daddr, masked,
+ set_ipv6_addr(skb, flow_key->ip.proto, masked, false,
recalc_csum);
memcpy(&flow_key->ipv6.addr.dst, masked,
sizeof(flow_key->ipv6.addr.dst));
+ nh = ipv6_hdr(skb);
}
}
if (mask->ipv6_tclass) {
base-commit: 2800868f63227c8237d70c41e70d441c919620d6
--
2.55.0.openai.911.g06873a30d2f3
reply other threads:[~2026-11-13 23:22 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20261113232158.61174-1-kylebot@openai.com \
--to=kylebot@openai.com \
--cc=aconole@redhat.com \
--cc=davem@davemloft.net \
--cc=dev@openvswitch.org \
--cc=echaudro@redhat.com \
--cc=edumazet@kernel.org \
--cc=horms@kernel.org \
--cc=i.maximets@ovn.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=stable@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox