get_ipv6_ext_hdrs() walks the IPv6 extension header chain to fill
key->ipv6.exthdrs, but advances to the next header with ipv6_optlen()
whatever the current header is. That is only right for Hop-by-Hop,
Destination Options and Routing headers:
- an Authentication Header is ((hdrlen + 2) << 2) bytes long
(ipv6_authlen()), so with hdrlen = 4 the walk skips 40 bytes instead
of 24 and reads the next "header" from the ICV and payload;
- a Fragment header is always 8 bytes, and the byte read as hdrlen is
its Reserved field, which the sender controls and receivers ignore,
so a non-zero value makes the walk skip up to 2048 bytes and miss
the headers that follow.
In both cases the OFPIEH12_* bits stored in key->ipv6.exthdrs depend on
bytes chosen by the sender rather than on the headers that are actually
present: payload bytes that look like another AH or a Hop-by-Hop header
add OFPIEH12_UNREP/OFPIEH12_UNSEQ, and real headers after a Fragment
header can go unreported. Flows that match on OVS_KEY_ATTR_IPV6_EXTHDRS
then classify such packets wrongly. There is no memory safety issue,
skb_header_pointer() bounds every read.
Advance by sizeof(struct frag_hdr) for Fragment headers and by
ipv6_authlen() for AH, as ipv6_find_hdr() does.
Fixes: 28a3f0601727 ("net: openvswitch: IPv6: Add IPv6 extension header
support")
Link:
https://lore.kernel.org/all/[email protected]/
Cc: [email protected]
Signed-off-by: Joas Antonio dos Santos <[email protected]>
Assisted-by: Yaga Agent HackerSec Claude:claude-opus-5-5
---
Found while going through the pre-existing issues the Sashiko review of
"openvswitch: fix wrong flag value in get_ipv6_ext_hdrs()" pointed out
(Link above); Eelco mentioned putting them on his todo list, and I did
not find a fix for this part in net, net-next or on the list. Happy to
drop this if a fix is already in flight.
Tested by building get_ipv6_ext_hdrs() before and after this patch in a
userspace harness (skb_header_pointer() replaced by a bounds-checked
memcpy from a buffer) and feeding it crafted packets:
AH(hdrlen=4) + DSTOPTS + TCP, payload mimicking AH at old offset
before: AUTH DEST UNREP UNSEQ after: AUTH DEST UNSEQ
FRAG(Reserved=0xff) + DSTOPTS + ROUTING + TCP
before: DEST FRAG UNSEQ after: DEST FRAG ROUTER UNSEQ
FRAG(Reserved=0) + ROUTING + TCP, HOP + DST + ROUTING + TCP
unchanged
net/openvswitch/ builds cleanly with W=1 (arm64, net/main 94cea342e891).
I did not test it on a live datapath.
net/openvswitch/flow.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 1c4f3a079..a1c60d2f5 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -372,8 +372,13 @@ static void get_ipv6_ext_hdrs(struct sk_buff *skb, struct
ipv6hdr *nh,
hp = skb_header_pointer(skb, start, sizeof(_hdr), &_hdr);
if (!hp)
break;
+ if (next_type == IPPROTO_FRAGMENT)
+ start += sizeof(struct frag_hdr);
+ else if (next_type == IPPROTO_AH)
+ start += ipv6_authlen(hp);
+ else
+ start += ipv6_optlen(hp);
next_type = hp->nexthdr;
- start += ipv6_optlen(hp);
}
}
--
2.39.5
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
