On 10/7/26 10:53 PM, Ilya Maximets wrote:
On 10/2/26 9:52 AM, Fernando Fernandez Mancera wrote:
When executing IPv6 address rewrite actions on IPv6 fragments,
set_ipv6_addr() calls update_ipv6_checksum(). If parse_ipv6hdr()
processes a non-first IPv6 fragment, it sets key->ip.proto to
NEXTHDR_FRAGMENT and returns early without calling
skb_set_transport_header().
update_ipv6_checksum() unconditionally evaluates skb_transport_offset()
on entry before checking l4_proto. Because skb->transport_header is
uninitialized, this triggers a warning under CONFIG_DEBUG_NET=y although
it is completely harmless.
Fix this by returning early in update_ipv6_checksum() if l4_proto is
NEXTHDR_FRAGMENT. This avoids reading the uninitialized transport offset
for fragments while preserving the debug warning for any other protocol
where the transport header is unexpectedly missing.
See the syzbot trace:
!skb_transport_header_was_set(skb)
WARNING: ./include/linux/skbuff.h:3075 at skb_transport_header
include/linux/skbuff.h:3075 [inline], CPU#1: syz-executor463/5635
WARNING: ./include/linux/skbuff.h:3075 at skb_transport_offset
include/linux/skbuff.h:3250 [inline], CPU#1: syz-executor463/5635
WARNING: ./include/linux/skbuff.h:3075 at update_ipv6_checksum
net/openvswitch/actions.c:361 [inline], CPU#1: syz-executor463/5635
WARNING: ./include/linux/skbuff.h:3075 at set_ipv6_addr+0x462/0x660
net/openvswitch/actions.c:399, CPU#1: syz-executor463/5635
[...]
RIP: 0010:skb_transport_header include/linux/skbuff.h:3075 [inline]
RIP: 0010:skb_transport_offset include/linux/skbuff.h:3250 [inline]
RIP: 0010:update_ipv6_checksum net/openvswitch/actions.c:361 [inline]
RIP: 0010:set_ipv6_addr+0x462/0x660 net/openvswitch/actions.c:399
[...]
Call Trace:
<TASK>
set_ipv6 net/openvswitch/actions.c:531 [inline]
do_execute_actions+0x557e/0x8600 net/openvswitch/actions.c:1366
ovs_execute_actions+0xde/0x520 net/openvswitch/actions.c:1592
ovs_packet_cmd_execute+0xb4f/0xf10 net/openvswitch/datapath.c:705
genl_family_rcv_msg_doit+0x233/0x340 net/netlink/genetlink.c:1114
genl_rcv_msg+0x614/0x7a0 net/netlink/genetlink.c:1209
netlink_rcv_skb+0x226/0x4a0 net/netlink/af_netlink.c:2572
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1218
netlink_unicast+0x7bd/0x940 net/netlink/af_netlink.c:1361
netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1916
sock_sendmsg_nosec+0x14e/0x190 net/socket.c:800
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=4cc63fcfb3845e149969
Fixes: 3fdbd1ce11e5 ("openvswitch: add ipv6 'set' action")
Signed-off-by: Fernando Fernandez Mancera <[email protected]>
---
net/openvswitch/actions.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index dc5ff859f114..68b42e900c73 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -358,7 +358,15 @@ static void set_ip_addr(struct sk_buff *skb, struct iphdr
*nh,
static void update_ipv6_checksum(struct sk_buff *skb, u8 l4_proto,
__be32 addr[4], const __be32 new_addr[4])
{
- int transport_len = skb->len - skb_transport_offset(skb);
+ int transport_len;
+
+ /* avoid reading the transport header offset if it isn't set,
+ * as it triggers a warning
+ */
nit: We prefer full sentences, i.e. start with a capital and end with a dot.
But also, I think, since the switch to NEXTHDR_FRAGMENT check, the comment
lost it's intended purpose as the code is pretty much self-documenting now.
It's clear that the fragment doesn't have the transport header. The comment
made sense if we needed to explain in which case we can get here without
having the offset initialized. I'd suggest we drop the comment.
You're also not adding such comments in the other patch for ipv4.
Fair, I thought that in IPv4 case it was more obvious than here. Anyway,
let me just drop it in a v3. Thanks a lot for the reviews :-)
Otherwise, LGTM.
+ 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)))
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
