--- /dev/null
+From 1f30fb9166d4f15a1aa19449b9da871fe0ed4796 Mon Sep 17 00:00:00 2001
+From: Ilya Maximets <i.maximets@ovn.org>
+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 <i.maximets@ovn.org>
+
+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 <stgraber@ubuntu.com>
+Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
+Acked-by: Aaron Conole <aconole@redhat.com>
+Signed-off-by: David S. Miller <davem@davemloft.net>
+[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 <gregkh@linuxfoundation.org>
+
+---
+ 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);
+ }
+
--- /dev/null
+From 2061ecfdf2350994e5b61c43e50e98a7a70e95ee Mon Sep 17 00:00:00 2001
+From: Ilya Maximets <i.maximets@ovn.org>
+Date: Tue, 7 Jun 2022 00:11:40 +0200
+Subject: net: openvswitch: fix misuse of the cached connection on tuple changes
+
+From: Ilya Maximets <i.maximets@ovn.org>
+
+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 <frode.nordahl@canonical.com>
+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 <i.maximets@ovn.org>
+Link: https://lore.kernel.org/r/20220606221140.488984-1-i.maximets@ovn.org
+Signed-off-by: Jakub Kicinski <kuba@kernel.org>
+[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 <gregkh@linuxfoundation.org>
+---
+ 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;
projects / thirdparty / kernel / stable-queue.git / commitdiff
