From patchwork Mon Jan 18 17:03:48 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hannes Frederic Sowa X-Patchwork-Id: 569612 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 6822D14076E for ; Tue, 19 Jan 2016 04:04:00 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=stressinduktion.org header.i=@stressinduktion.org header.b=k9CrYRji; dkim=pass (1024-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b=RvGp/kSM; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755831AbcARRD5 (ORCPT ); Mon, 18 Jan 2016 12:03:57 -0500 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:33714 "EHLO out5-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755597AbcARRDz (ORCPT ); Mon, 18 Jan 2016 12:03:55 -0500 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 2C9F8209DA for ; Mon, 18 Jan 2016 12:03:55 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute5.internal (MEProxy); Mon, 18 Jan 2016 12:03:55 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= stressinduktion.org; h=cc:date:from:message-id:subject:to :x-sasl-enc:x-sasl-enc; s=mesmtp; bh=m0WQVkaN9j2fqS2VOAiXbF/JlgE =; b=k9CrYRjimasoKu26hFcWBkp5PYHsYjuc98oD9QEMRXzwNHgJhr9GwKz2bc+ 5pCH8NJV1LLX6ilIMJta6ezEoNw6QBIA/iqVaMgfTpvHJfBQkTaA34dFKHB6jDmo s2gVROlpVGNcvBR/eqMmOBG98z946Ss2WcyxcHxHE1CyOUJM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:date:from:message-id:subject:to :x-sasl-enc:x-sasl-enc; s=smtpout; bh=m0WQVkaN9j2fqS2VOAiXbF/Jlg E=; b=RvGp/kSMaN3JViRu1qQCgp70tj3GwN7u8+IOMPvvdqOzcjheAofLvQA9/X 7wS6l8GFj670luutRzs7onE7CxJDXgE/ssp4qAvpM7zgNEfocYWC21eOrirUoZVy /J5zbwUeSDSeaKvQIl2EF93wa0d6VyX2ycUweHStXsOYHCR/w= X-Sasl-enc: sX9kLM1r/CTXX+EfcSzESS2iPQWSRvGbrLA+bk889Swl 1453136634 Received: from z.localhost.localdomain (unknown [217.192.177.51]) by mail.messagingengine.com (Postfix) with ESMTPA id 5C3956800D5; Mon, 18 Jan 2016 12:03:54 -0500 (EST) From: Hannes Frederic Sowa To: netdev@vger.kernel.org Cc: Pravin Shelar , Simon Horman , Eric Dumazet Subject: [PATCH net v5] ovs: limit ovs recursions in ovs_execute_actions to not corrupt stack Date: Mon, 18 Jan 2016 18:03:48 +0100 Message-Id: <1453136628-9816-1-git-send-email-hannes@stressinduktion.org> X-Mailer: git-send-email 2.5.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Cc: Simon Horman Cc: Eric Dumazet Cc: Simon Horman Signed-off-by: Hannes Frederic Sowa --- 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; }