netdev - [PATCH net v5] ovs: limit ovs recursions in ovs_execute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1453136628-9816-1-git-send-email-hannes@stressinduktion.org>
Date:	Mon, 18 Jan 2016 18:03:48 +0100
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	netdev@...r.kernel.org
Cc:	Pravin Shelar <pshelar@....org>,
	Simon Horman <simon.horman@...ronome.com>,
	Eric Dumazet <eric.dumazet@...il.com>
Subject: [PATCH net v5] ovs: limit ovs recursions in ovs_execute_actions to not corrupt stack

It was seen that defective configurations of openvswitch could overwrite
the STACK_END_MAGIC and cause a hard crash of the kernel because of too
many recursions within ovs.

This problem arises due to the high stack usage of openvswitch. The rest
of the kernel is fine with the current limit of 10 (RECURSION_LIMIT).

We use the already existing recursion counter in ovs_execute_actions to
implement an upper bound of 5 recursions.

Cc: Pravin Shelar <pshelar@....org>
Cc: Simon Horman <simon.horman@...ronome.com>
Cc: Eric Dumazet <eric.dumazet@...il.com>
Cc: Simon Horman <simon.horman@...ronome.com>
Signed-off-by: Hannes Frederic Sowa <hannes@...essinduktion.org>
---
v2) added preemption guards
v3) Pravin suggested to reuse the ovs_execute_actions counter which this
    patch does. Also only allow 5 recursions as suggested by Pravin.
v4) added unlikely as suggested by Eric
v5) removed preempt guards as brought up with Pravin and discussed with David

Thanks to all reviewers!

 net/openvswitch/actions.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index c88d0f2d3e019b..2d59df52191574 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1160,17 +1160,26 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
 			const struct sw_flow_actions *acts,
 			struct sw_flow_key *key)
 {
-	int level = this_cpu_read(exec_actions_level);
-	int err;
+	static const int ovs_recursion_limit = 5;
+	int err, level;
+
+	level = __this_cpu_inc_return(exec_actions_level);
+	if (unlikely(level > ovs_recursion_limit)) {
+		net_crit_ratelimited("ovs: recursion limit reached on datapath %s, probable configuration error\n",
+				     ovs_dp_name(dp));
+		kfree_skb(skb);
+		err = -ENETDOWN;
+		goto out;
+	}
 
-	this_cpu_inc(exec_actions_level);
 	err = do_execute_actions(dp, skb, key,
 				 acts->actions, acts->actions_len);
 
-	if (!level)
+	if (level == 1)
 		process_deferred_actions(dp);
 
-	this_cpu_dec(exec_actions_level);
+out:
+	__this_cpu_dec(exec_actions_level);
 	return err;
 }
 
-- 
2.5.0