Currently, check_iphdr() sets the transport header offset for all IPv4 packets, including later fragments that do not contain an L4 header.
As discussed, let's not set the transport header for non-first IPv4 fragments to prevent a future out-of-bounds memory access that can happen silently because the warning at skb_transport_header_offset() does not trigger. In addition, defer the transport_len calculation until after the fragment check. Link: https://lore.kernel.org/netdev/[email protected]/ Suggested-by: Ilya Maximets <[email protected]> Signed-off-by: Fernando Fernandez Mancera <[email protected]> --- Sashiko: If you notice a similar problem at IPv6 path, we have fixed it already in net tree, here is submission: https://lore.kernel.org/netdev/[email protected]/ --- net/openvswitch/actions.c | 4 +++- net/openvswitch/flow.c | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c index dc5ff859f114..867c26e14ead 100644 --- a/net/openvswitch/actions.c +++ b/net/openvswitch/actions.c @@ -322,11 +322,13 @@ static int pop_nsh(struct sk_buff *skb, struct sw_flow_key *key) static void update_ip_l4_checksum(struct sk_buff *skb, struct iphdr *nh, __be32 addr, __be32 new_addr) { - int transport_len = skb->len - skb_transport_offset(skb); + int transport_len; if (nh->frag_off & htons(IP_OFFSET)) return; + transport_len = skb->len - skb_transport_offset(skb); + if (nh->protocol == IPPROTO_TCP) { if (likely(transport_len >= sizeof(struct tcphdr))) inet_proto_csum_replace4(&tcp_hdr(skb)->check, skb, diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index 1c4f3a079044..d549a58180a7 100644 --- a/net/openvswitch/flow.c +++ b/net/openvswitch/flow.c @@ -189,6 +189,7 @@ static bool arphdr_ok(struct sk_buff *skb) static int check_iphdr(struct sk_buff *skb) { unsigned int nh_ofs = skb_network_offset(skb); + const struct iphdr *nh; unsigned int ip_len; int err; @@ -201,7 +202,11 @@ static int check_iphdr(struct sk_buff *skb) skb->len < nh_ofs + ip_len)) return -EINVAL; - skb_set_transport_header(skb, nh_ofs + ip_len); + nh = ip_hdr(skb); + + if (!(nh->frag_off & htons(IP_OFFSET))) + skb_set_transport_header(skb, nh_ofs + ip_len); + return 0; } -- 2.55.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
