Netdev List
 help / color / mirror / Atom feed
From: "zhounan (E)" <zhounan14@huawei.com>
To: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"dev@openvswitch.org" <dev@openvswitch.org>,
	"bugs@openvswitch.org" <bugs@openvswitch.org>
Cc: "liucheng (J)" <liucheng11@huawei.com>,
	"Hejiajun (he jiajun, SOCF&uDF )" <hejiajun@huawei.com>,
	Lichunhe <lichunhe@huawei.com>,
	Gregory Rose <gvrose8192@gmail.com>,
	"pravin.ovn@gmail.com" <pravin.ovn@gmail.com>
Subject: [ovs-dev] [PATCH] datapath: fix crash when ipv6 fragment pkt recalculate L4 checksum
Date: Fri, 10 Dec 2021 02:59:32 +0000	[thread overview]
Message-ID: <396da6f61fa948ac854531e935921dfc@huawei.com> (raw)
In-Reply-To: <35aa84e0d1fe4bd1ad1bf6fb61c83338@huawei.com>

From: Zhou Nan <zhounan14@huawei.com>

When we set ipv6 addr, we need to recalculate checksum of L4 header.
In our testcase, after send ipv6 fragment package, KASAN detect "use after free" when calling function update_ipv6_checksum, and crash occurred after a while.
If ipv6 package is fragment, and it is not first seg, we should not recalculate checksum of L4 header since this kind of package has no
L4 header.
To prevent crash, we set "recalc_csum" "false" when calling function "set_ipv6_addr".
We also find that function skb_ensure_writable (make sure L4 header is writable) is helpful before calling inet_proto_csum_replace16 to recalculate checksum.

Fixes: ada5efce102d6191e5c66fc385ba52a2d340ef50
       ("datapath: Fix IPv6 later frags parsing")

Signed-off-by: Zhou Nan <zhounan14@huawei.com>
---
 datapath/actions.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/datapath/actions.c b/datapath/actions.c index fbf4457..52cf03e 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -456,12 +456,21 @@ 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 err;
 
 	if (l4_proto == NEXTHDR_TCP) {
+		err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+				sizeof(struct tcphdr));
+		if (unlikely(err))
+			return;
 		if (likely(transport_len >= sizeof(struct tcphdr)))
 			inet_proto_csum_replace16(&tcp_hdr(skb)->check, skb,
 						  addr, new_addr, true);
 	} else if (l4_proto == NEXTHDR_UDP) {
+		err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+				sizeof(struct udphdr));
+		if (unlikely(err))
+			return;
 		if (likely(transport_len >= sizeof(struct udphdr))) {
 			struct udphdr *uh = udp_hdr(skb);
 
@@ -473,6 +482,10 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 l4_proto,
 			}
 		}
 	} else if (l4_proto == NEXTHDR_ICMP) {
+		err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+				sizeof(struct icmp6hdr));
+		if (unlikely(err))
+			return;
 		if (likely(transport_len >= sizeof(struct icmp6hdr)))
 			inet_proto_csum_replace16(&icmp6_hdr(skb)->icmp6_cksum,
 						  skb, addr, new_addr, true);
@@ -589,12 +602,15 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *flow_key,
 	if (is_ipv6_mask_nonzero(mask->ipv6_src)) {
 		__be32 *saddr = (__be32 *)&nh->saddr;
 		__be32 masked[4];
+		bool recalc_csum = true;
 
 		mask_ipv6_addr(saddr, key->ipv6_src, mask->ipv6_src, masked);
 
 		if (unlikely(memcmp(saddr, masked, sizeof(masked)))) {
+			if (flow_key->ip.frag == OVS_FRAG_TYPE_LATER)
+				recalc_csum = false;
 			set_ipv6_addr(skb, flow_key->ip.proto, saddr, masked,
-				      true);
+				      recalc_csum);
 			memcpy(&flow_key->ipv6.addr.src, masked,
 			       sizeof(flow_key->ipv6.addr.src));
 		}
@@ -614,6 +630,8 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *flow_key,
 							     NEXTHDR_ROUTING,
 							     NULL, &flags)
 					       != NEXTHDR_ROUTING);
+			if (flow_key->ip.frag == OVS_FRAG_TYPE_LATER)
+				recalc_csum = false;
 
 			set_ipv6_addr(skb, flow_key->ip.proto, daddr, masked,
 				      recalc_csum);
--
2.27.0


       reply	other threads:[~2021-12-10  2:59 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <54c2ae5d003f49f6a29eec6a67c72315@huawei.com>
     [not found] ` <35aa84e0d1fe4bd1ad1bf6fb61c83338@huawei.com>
2021-12-10  2:59   ` zhounan (E) [this message]
2021-12-10  4:48     ` [ovs-discuss] [ovs-dev] [PATCH] datapath: fix crash when ipv6 fragment pkt recalculate L4 checksum Tonghao Zhang

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=396da6f61fa948ac854531e935921dfc@huawei.com \
    --to=zhounan14@huawei.com \
    --cc=bugs@openvswitch.org \
    --cc=dev@openvswitch.org \
    --cc=gvrose8192@gmail.com \
    --cc=hejiajun@huawei.com \
    --cc=lichunhe@huawei.com \
    --cc=liucheng11@huawei.com \
    --cc=netdev@vger.kernel.org \
    --cc=pravin.ovn@gmail.com \
    /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