From patchwork Mon Sep 11 19:56:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christophe JAILLET X-Patchwork-Id: 812576 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xrf121wDTz9s8J for ; Tue, 12 Sep 2017 06:00:22 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 235FDC04; Mon, 11 Sep 2017 19:59:58 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 6D3AABEE for ; Mon, 11 Sep 2017 19:59:56 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from smtp.smtpout.orange.fr (smtp07.smtpout.orange.fr [80.12.242.129]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 9A43443A for ; Mon, 11 Sep 2017 19:59:55 +0000 (UTC) Received: from localhost.localdomain ([86.196.182.67]) by mwinf5d13 with ME id 8Kzt1w00A1TfVo603Kztnr; Mon, 11 Sep 2017 21:59:53 +0200 X-ME-Helo: localhost.localdomain X-ME-Auth: Y2hyaXN0b3BoZS5qYWlsbGV0QHdhbmFkb28uZnI= X-ME-Date: Mon, 11 Sep 2017 21:59:53 +0200 X-ME-IP: 86.196.182.67 From: Christophe JAILLET To: pshelar@nicira.com, davem@davemloft.net, xiangxia.m.yue@gmail.com Date: Mon, 11 Sep 2017 21:56:20 +0200 Message-Id: <20170911195620.17931-1-christophe.jaillet@wanadoo.fr> X-Mailer: git-send-email 2.11.0 X-Spam-Status: No, score=0.0 required=5.0 tests=FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: dev@openvswitch.org, netdev@vger.kernel.org, Christophe JAILLET , kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [ovs-dev] [PATCH v2] openvswitch: Fix an error handling path in 'ovs_nla_init_match_and_action()' X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org All other error handling paths in this function go through the 'error' label. This one should do the same. Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.") Signed-off-by: Christophe JAILLET Acked-by: Pravin B Shelar --- I think that the comment above the function could be improved. It looks like the commit log which has introduced this function. I'm also not sure that commit 9cc9a5cb176c is of any help. It is supposed to remove a warning, and I guess it does. But 'ovs_nla_init_match_and_action()' is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack used by each function is reduced, the overall stack should be the same, if not larger. So this commit sounds like adding a bug where the code was fine and states to fix an issue but, at the best, only hides it. Instead of fixing the code with the proposed patch, reverting the initial commit could also be considered. V2: update Subject line --- net/openvswitch/datapath.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index 76cf273a56c7..c3aec6227c91 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net, if (!a[OVS_FLOW_ATTR_KEY]) { OVS_NLERR(log, "Flow key attribute not present in set flow."); - return -EINVAL; + error = -EINVAL; + goto error; } *acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,