From: Greg Kroah-Hartman Date: Mon, 20 Jun 2022 11:56:38 +0000 (+0200) Subject: 4.19-stable patches X-Git-Tag: v5.4.200~13 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f4f4a84827bba151bb6d04bc5cc212d35fc4d7bb;p=thirdparty%2Fkernel%2Fstable-queue.git 4.19-stable patches added patches: net-openvswitch-fix-leak-of-nested-actions.patch net-openvswitch-fix-misuse-of-the-cached-connection-on-tuple-changes.patch --- diff --git a/queue-4.19/net-openvswitch-fix-leak-of-nested-actions.patch b/queue-4.19/net-openvswitch-fix-leak-of-nested-actions.patch new file mode 100644 index 00000000000..6eb1e1f6a8c --- /dev/null +++ b/queue-4.19/net-openvswitch-fix-leak-of-nested-actions.patch @@ -0,0 +1,150 @@ +From 1f30fb9166d4f15a1aa19449b9da871fe0ed4796 Mon Sep 17 00:00:00 2001 +From: Ilya Maximets +Date: Mon, 4 Apr 2022 17:43:45 +0200 +Subject: net: openvswitch: fix leak of nested actions +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Ilya Maximets + +commit 1f30fb9166d4f15a1aa19449b9da871fe0ed4796 upstream. + +While parsing user-provided actions, openvswitch module may dynamically +allocate memory and store pointers in the internal copy of the actions. +So this memory has to be freed while destroying the actions. + +Currently there are only two such actions: ct() and set(). However, +there are many actions that can hold nested lists of actions and +ovs_nla_free_flow_actions() just jumps over them leaking the memory. + +For example, removal of the flow with the following actions will lead +to a leak of the memory allocated by nf_ct_tmpl_alloc(): + + actions:clone(ct(commit),0) + +Non-freed set() action may also leak the 'dst' structure for the +tunnel info including device references. + +Under certain conditions with a high rate of flow rotation that may +cause significant memory leak problem (2MB per second in reporter's +case). The problem is also hard to mitigate, because the user doesn't +have direct control over the datapath flows generated by OVS. + +Fix that by iterating over all the nested actions and freeing +everything that needs to be freed recursively. + +New build time assertion should protect us from this problem if new +actions will be added in the future. + +Unfortunately, openvswitch module doesn't use NLA_F_NESTED, so all +attributes has to be explicitly checked. sample() and clone() actions +are mixing extra attributes into the user-provided action list. That +prevents some code generalization too. + +Fixes: 34ae932a4036 ("openvswitch: Make tunnel set action attach a metadata dst") +Link: https://mail.openvswitch.org/pipermail/ovs-dev/2022-March/392922.html +Reported-by: Stéphane Graber +Signed-off-by: Ilya Maximets +Acked-by: Aaron Conole +Signed-off-by: David S. Miller +[Backport for 4.19: Removed handling of OVS_ACTION_ATTR_DEC_TTL + and OVS_ACTION_ATTR_CHECK_PKT_LEN as these actions do not exist + in this version. BUILD_BUG_ON condition adjusted accordingly.] +Signed-off-by: Greg Kroah-Hartman + +--- + net/openvswitch/flow_netlink.c | 61 +++++++++++++++++++++++++++++++++++++---- + 1 file changed, 56 insertions(+), 5 deletions(-) + +--- a/net/openvswitch/flow_netlink.c ++++ b/net/openvswitch/flow_netlink.c +@@ -2253,6 +2253,36 @@ static struct sw_flow_actions *nla_alloc + return sfa; + } + ++static void ovs_nla_free_nested_actions(const struct nlattr *actions, int len); ++ ++static void ovs_nla_free_clone_action(const struct nlattr *action) ++{ ++ const struct nlattr *a = nla_data(action); ++ int rem = nla_len(action); ++ ++ switch (nla_type(a)) { ++ case OVS_CLONE_ATTR_EXEC: ++ /* The real list of actions follows this attribute. */ ++ a = nla_next(a, &rem); ++ ovs_nla_free_nested_actions(a, rem); ++ break; ++ } ++} ++ ++static void ovs_nla_free_sample_action(const struct nlattr *action) ++{ ++ const struct nlattr *a = nla_data(action); ++ int rem = nla_len(action); ++ ++ switch (nla_type(a)) { ++ case OVS_SAMPLE_ATTR_ARG: ++ /* The real list of actions follows this attribute. */ ++ a = nla_next(a, &rem); ++ ovs_nla_free_nested_actions(a, rem); ++ break; ++ } ++} ++ + static void ovs_nla_free_set_action(const struct nlattr *a) + { + const struct nlattr *ovs_key = nla_data(a); +@@ -2266,25 +2296,46 @@ static void ovs_nla_free_set_action(cons + } + } + +-void ovs_nla_free_flow_actions(struct sw_flow_actions *sf_acts) ++static void ovs_nla_free_nested_actions(const struct nlattr *actions, int len) + { + const struct nlattr *a; + int rem; + +- if (!sf_acts) ++ /* Whenever new actions are added, the need to update this ++ * function should be considered. ++ */ ++ BUILD_BUG_ON(OVS_ACTION_ATTR_MAX != 20); ++ ++ if (!actions) + return; + +- nla_for_each_attr(a, sf_acts->actions, sf_acts->actions_len, rem) { ++ nla_for_each_attr(a, actions, len, rem) { + switch (nla_type(a)) { +- case OVS_ACTION_ATTR_SET: +- ovs_nla_free_set_action(a); ++ case OVS_ACTION_ATTR_CLONE: ++ ovs_nla_free_clone_action(a); + break; ++ + case OVS_ACTION_ATTR_CT: + ovs_ct_free_action(a); + break; ++ ++ case OVS_ACTION_ATTR_SAMPLE: ++ ovs_nla_free_sample_action(a); ++ break; ++ ++ case OVS_ACTION_ATTR_SET: ++ ovs_nla_free_set_action(a); ++ break; + } + } ++} ++ ++void ovs_nla_free_flow_actions(struct sw_flow_actions *sf_acts) ++{ ++ if (!sf_acts) ++ return; + ++ ovs_nla_free_nested_actions(sf_acts->actions, sf_acts->actions_len); + kfree(sf_acts); + } + diff --git a/queue-4.19/net-openvswitch-fix-misuse-of-the-cached-connection-on-tuple-changes.patch b/queue-4.19/net-openvswitch-fix-misuse-of-the-cached-connection-on-tuple-changes.patch new file mode 100644 index 00000000000..217f81daf22 --- /dev/null +++ b/queue-4.19/net-openvswitch-fix-misuse-of-the-cached-connection-on-tuple-changes.patch @@ -0,0 +1,107 @@ +From 2061ecfdf2350994e5b61c43e50e98a7a70e95ee Mon Sep 17 00:00:00 2001 +From: Ilya Maximets +Date: Tue, 7 Jun 2022 00:11:40 +0200 +Subject: net: openvswitch: fix misuse of the cached connection on tuple changes + +From: Ilya Maximets + +commit 2061ecfdf2350994e5b61c43e50e98a7a70e95ee upstream. + +If packet headers changed, the cached nfct is no longer relevant +for the packet and attempt to re-use it leads to the incorrect packet +classification. + +This issue is causing broken connectivity in OpenStack deployments +with OVS/OVN due to hairpin traffic being unexpectedly dropped. + +The setup has datapath flows with several conntrack actions and tuple +changes between them: + + actions:ct(commit,zone=8,mark=0/0x1,nat(src)), + set(eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:06)), + set(ipv4(src=172.18.2.10,dst=192.168.100.6,ttl=62)), + ct(zone=8),recirc(0x4) + +After the first ct() action the packet headers are almost fully +re-written. The next ct() tries to re-use the existing nfct entry +and marks the packet as invalid, so it gets dropped later in the +pipeline. + +Clearing the cached conntrack entry whenever packet tuple is changed +to avoid the issue. + +The flow key should not be cleared though, because we should still +be able to match on the ct_state if the recirculation happens after +the tuple change but before the next ct() action. + +Cc: stable@vger.kernel.org +Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action") +Reported-by: Frode Nordahl +Link: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-May/051829.html +Link: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/1967856 +Signed-off-by: Ilya Maximets +Link: https://lore.kernel.org/r/20220606221140.488984-1-i.maximets@ovn.org +Signed-off-by: Jakub Kicinski +[Backport to 5.10: minor rebase in ovs_ct_clear function. + This version also applicable to and tested on 5.4 and 4.19.] +Signed-off-by: Greg Kroah-Hartman +--- + net/openvswitch/actions.c | 6 ++++++ + net/openvswitch/conntrack.c | 3 ++- + 2 files changed, 8 insertions(+), 1 deletion(-) + +--- a/net/openvswitch/actions.c ++++ b/net/openvswitch/actions.c +@@ -443,6 +443,7 @@ static void set_ip_addr(struct sk_buff * + update_ip_l4_checksum(skb, nh, *addr, new_addr); + csum_replace4(&nh->check, *addr, new_addr); + skb_clear_hash(skb); ++ ovs_ct_clear(skb, NULL); + *addr = new_addr; + } + +@@ -490,6 +491,7 @@ static void set_ipv6_addr(struct sk_buff + update_ipv6_checksum(skb, l4_proto, addr, new_addr); + + skb_clear_hash(skb); ++ ovs_ct_clear(skb, NULL); + memcpy(addr, new_addr, sizeof(__be32[4])); + } + +@@ -730,6 +732,7 @@ static int set_nsh(struct sk_buff *skb, + static void set_tp_port(struct sk_buff *skb, __be16 *port, + __be16 new_port, __sum16 *check) + { ++ ovs_ct_clear(skb, NULL); + inet_proto_csum_replace2(check, skb, *port, new_port, false); + *port = new_port; + } +@@ -769,6 +772,7 @@ static int set_udp(struct sk_buff *skb, + uh->dest = dst; + flow_key->tp.src = src; + flow_key->tp.dst = dst; ++ ovs_ct_clear(skb, NULL); + } + + skb_clear_hash(skb); +@@ -831,6 +835,8 @@ static int set_sctp(struct sk_buff *skb, + sh->checksum = old_csum ^ old_correct_csum ^ new_csum; + + skb_clear_hash(skb); ++ ovs_ct_clear(skb, NULL); ++ + flow_key->tp.src = sh->source; + flow_key->tp.dst = sh->dest; + +--- a/net/openvswitch/conntrack.c ++++ b/net/openvswitch/conntrack.c +@@ -1303,7 +1303,8 @@ int ovs_ct_clear(struct sk_buff *skb, st + if (skb_nfct(skb)) { + nf_conntrack_put(skb_nfct(skb)); + nf_ct_set(skb, NULL, IP_CT_UNTRACKED); +- ovs_ct_fill_key(skb, key); ++ if (key) ++ ovs_ct_fill_key(skb, key); + } + + return 0; diff --git a/queue-4.19/series b/queue-4.19/series index c34cf344953..d8586f177e8 100644 --- a/queue-4.19/series +++ b/queue-4.19/series @@ -220,3 +220,5 @@ ext4-fix-bug_on-ext4_mb_use_inode_pa.patch ext4-make-variable-count-signed.patch ext4-add-reserved-gdt-blocks-check.patch virtio-pci-remove-wrong-address-verification-in-vp_del_vqs.patch +net-openvswitch-fix-misuse-of-the-cached-connection-on-tuple-changes.patch +net-openvswitch-fix-leak-of-nested-actions.patch