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.
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
