From patchwork Wed Mar 6 15:50:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vlad Buslov X-Patchwork-Id: 1052404 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=mellanox.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 44Dysj16Pxz9sMx for ; Thu, 7 Mar 2019 02:51:04 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729836AbfCFPvD (ORCPT ); Wed, 6 Mar 2019 10:51:03 -0500 Received: from mail-il-dmz.mellanox.com ([193.47.165.129]:56837 "EHLO mellanox.co.il" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1729823AbfCFPvC (ORCPT ); Wed, 6 Mar 2019 10:51:02 -0500 Received: from Internal Mail-Server by MTLPINE1 (envelope-from vladbu@mellanox.com) with ESMTPS (AES256-SHA encrypted); 6 Mar 2019 17:50:55 +0200 Received: from reg-r-vrt-018-180.mtr.labs.mlnx. (reg-r-vrt-018-180.mtr.labs.mlnx [10.213.18.180]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id x26FotCQ029524; Wed, 6 Mar 2019 17:50:55 +0200 From: Vlad Buslov To: netdev@vger.kernel.org Cc: jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net, Vlad Buslov Subject: [PATCH net-next] net: sched: fix potential use-after-free in __tcf_chain_put() Date: Wed, 6 Mar 2019 17:50:43 +0200 Message-Id: <20190306155043.3768-1-vladbu@mellanox.com> X-Mailer: git-send-email 2.13.6 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org When used with unlocked classifier that have filters attached to actions with goto chain, __tcf_chain_put() for last non action reference can race with calls to same function from action cleanup code that releases last action reference. In this case action cleanup handler could free the chain if it executes after all references to chain were released, but before all concurrent users finished using it. Modify __tcf_chain_put() to only access tcf_chain fields when holding block->lock. Remove local variables that were used to cache some tcf_chain fields and are no longer needed because their values can now be obtained directly from chain under block->lock protection. Fixes: 726d061286ce ("net: sched: prevent insertion of new classifiers during chain flush") Signed-off-by: Vlad Buslov --- net/sched/cls_api.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 478095d50f95..2c2aac4ac721 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -470,10 +470,9 @@ static void __tcf_chain_put(struct tcf_chain *chain, bool by_act, { struct tcf_block *block = chain->block; const struct tcf_proto_ops *tmplt_ops; - bool is_last, free_block = false; + bool free_block = false; unsigned int refcnt; void *tmplt_priv; - u32 chain_index; mutex_lock(&block->lock); if (explicitly_created) { @@ -492,23 +491,21 @@ static void __tcf_chain_put(struct tcf_chain *chain, bool by_act, * save these to temporary variables. */ refcnt = --chain->refcnt; - is_last = refcnt - chain->action_refcnt == 0; tmplt_ops = chain->tmplt_ops; tmplt_priv = chain->tmplt_priv; - chain_index = chain->index; - - if (refcnt == 0) - free_block = tcf_chain_detach(chain); - mutex_unlock(&block->lock); /* The last dropped non-action reference will trigger notification. */ - if (is_last && !by_act) { - tc_chain_notify_delete(tmplt_ops, tmplt_priv, chain_index, + if (refcnt - chain->action_refcnt == 0 && !by_act) { + tc_chain_notify_delete(tmplt_ops, tmplt_priv, chain->index, block, NULL, 0, 0, false); /* Last reference to chain, no need to lock. */ chain->flushing = false; } + if (refcnt == 0) + free_block = tcf_chain_detach(chain); + mutex_unlock(&block->lock); + if (refcnt == 0) { tc_chain_tmplt_del(tmplt_ops, tmplt_priv); tcf_chain_destroy(chain, free_block);