* Re: [ovs-discuss] [ovs-dev] [PATCH] datapath: fix crash when ipv6 fragment pkt recalculate L4 checksum
2021-12-10 2:59 ` [ovs-dev] [PATCH] datapath: fix crash when ipv6 fragment pkt recalculate L4 checksum zhounan (E)
@ 2021-12-10 4:48 ` Tonghao Zhang
0 siblings, 0 replies; 2+ messages in thread
From: Tonghao Zhang @ 2021-12-10 4:48 UTC (permalink / raw)
To: zhounan (E)
Cc: netdev@vger.kernel.org, dev@openvswitch.org, bugs@openvswitch.org,
pravin.ovn@gmail.com, Lichunhe, liucheng (J),
Hejiajun (he jiajun, SOCF&uDF ), Greg Rose
On Fri, Dec 10, 2021 at 10:59 AM zhounan (E) via discuss
<ovs-discuss@openvswitch.org> wrote:
>
> 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
>
> _______________________________________________
> discuss mailing list
As Gregory said, you should rebase your patch on linux upstream. and
patch is reviewd in netdev@vger.kernel.org mail list.
OvS kernel module in upstream is:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/net/openvswitch
When the patch is applied in linux upstream, you can backport it.
Please see the section "Changes to Linux kernel components"
https://docs.openvswitch.org/en/latest/internals/contributing/backporting-patches/
> discuss@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
--
Best regards, Tonghao
^ permalink raw reply [flat|nested] 2+ messages in thread