netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Aaron Conole <aconole@redhat.com>
To: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Pravin B Shelar <pshelar@ovn.org>,
	dev@openvswitch.org, Ilya Maximets <i.maximets@ovn.org>,
	Simon Horman <horms@ovn.org>,
	Eelco Chaudron <echaudro@redhat.com>
Subject: [PATCH net 1/2] net: openvswitch: limit the number of recursions from action sets
Date: Tue,  6 Feb 2024 08:11:46 -0500	[thread overview]
Message-ID: <20240206131147.1286530-2-aconole@redhat.com> (raw)
In-Reply-To: <20240206131147.1286530-1-aconole@redhat.com>

The ovs module allows for some actions to recursively contain an action
list for complex scenarios, such as sampling, checking lengths, etc.
When these actions are copied into the internal flow table, they are
evaluated to validate that such actions make sense, and these calls
happen recursively.

The ovs-vswitchd userspace won't emit more than 16 recursion levels
deep.  However, the module has no such limit and will happily accept
limits larger than 16 levels nested.  Prevent this by tracking the
number of recursions happening and manually limiting it to 16 levels
nested.

The initial implementation of the sample action would track this depth
and prevent more than 3 levels of recursion, but this was removed to
support the clone use case, rather than limited at the current userspace
limit.

Fixes: 798c166173ff ("openvswitch: Optimize sample action for the clone use cases")
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 net/openvswitch/flow_netlink.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 88965e2068ac..ba5cfa67a720 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -48,6 +48,9 @@ struct ovs_len_tbl {
 
 #define OVS_ATTR_NESTED -1
 #define OVS_ATTR_VARIABLE -2
+#define OVS_COPY_ACTIONS_MAX_DEPTH 16
+
+static DEFINE_PER_CPU(int, copy_actions_depth);
 
 static bool actions_may_change_flow(const struct nlattr *actions)
 {
@@ -3148,11 +3151,11 @@ static int copy_action(const struct nlattr *from,
 	return 0;
 }
 
-static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
-				  const struct sw_flow_key *key,
-				  struct sw_flow_actions **sfa,
-				  __be16 eth_type, __be16 vlan_tci,
-				  u32 mpls_label_count, bool log)
+static int ___ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
+				   const struct sw_flow_key *key,
+				   struct sw_flow_actions **sfa,
+				   __be16 eth_type, __be16 vlan_tci,
+				   u32 mpls_label_count, bool log)
 {
 	u8 mac_proto = ovs_key_mac_proto(key);
 	const struct nlattr *a;
@@ -3478,6 +3481,26 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
 	return 0;
 }
 
+static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
+				  const struct sw_flow_key *key,
+				  struct sw_flow_actions **sfa,
+				  __be16 eth_type, __be16 vlan_tci,
+				  u32 mpls_label_count, bool log)
+{
+	int level = this_cpu_read(copy_actions_depth);
+	int ret;
+
+	if (level > OVS_COPY_ACTIONS_MAX_DEPTH)
+		return -EOVERFLOW;
+
+	__this_cpu_inc(copy_actions_depth);
+	ret = ___ovs_nla_copy_actions(net, attr, key, sfa, eth_type,
+				      vlan_tci, mpls_label_count, log);
+	__this_cpu_dec(copy_actions_depth);
+
+	return ret;
+}
+
 /* 'key' must be the masked key. */
 int ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
 			 const struct sw_flow_key *key,
-- 
2.41.0


  reply	other threads:[~2024-02-06 13:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-06 13:11 [PATCH net 0/2] net: openvswitch: limit the recursions from action sets Aaron Conole
2024-02-06 13:11 ` Aaron Conole [this message]
2024-02-06 14:30   ` [PATCH net 1/2] net: openvswitch: limit the number of " Eric Dumazet
2024-02-06 14:55     ` Aaron Conole
2024-02-06 14:56       ` Eric Dumazet
2024-02-06 15:40         ` Aaron Conole
2024-02-06 13:11 ` [PATCH net 2/2] selftests: openvswitch: Add validation for the recursion test Aaron Conole

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=20240206131147.1286530-2-aconole@redhat.com \
    --to=aconole@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dev@openvswitch.org \
    --cc=echaudro@redhat.com \
    --cc=edumazet@google.com \
    --cc=horms@ovn.org \
    --cc=i.maximets@ovn.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pshelar@ovn.org \
    /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;
as well as URLs for NNTP newsgroup(s).