From patchwork Thu Apr 20 19:33:58 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jon Maloy X-Patchwork-Id: 752986 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 3w88FT5dq3z9s78 for ; Fri, 21 Apr 2017 05:34:21 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S947094AbdDTTeU (ORCPT ); Thu, 20 Apr 2017 15:34:20 -0400 Received: from sesbmg23.ericsson.net ([193.180.251.37]:63073 "EHLO sesbmg23.ericsson.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S947061AbdDTTeS (ORCPT ); Thu, 20 Apr 2017 15:34:18 -0400 X-AuditID: c1b4fb25-837ff70000006af2-2b-58f90d38a659 Received: from ESESSHC011.ericsson.se (Unknown_Domain [153.88.183.51]) by (Symantec Mail Security) with SMTP id C9.B5.27378.83D09F85; Thu, 20 Apr 2017 21:34:17 +0200 (CEST) Received: from tipsy.lab.linux.ericsson.se (10.35.28.120) by ESESSHC011.ericsson.se (153.88.183.51) with Microsoft SMTP Server (TLS) id 14.3.339.0; Thu, 20 Apr 2017 21:34:15 +0200 From: Jon Maloy To: , CC: , , Subject: [PATCH stable 4.4] tipc: fix crash during node removal Date: Thu, 20 Apr 2017 21:33:58 +0200 Message-ID: <1492716838-9182-1-git-send-email-jon.maloy@ericsson.com> X-Mailer: git-send-email 2.1.4 MIME-Version: 1.0 X-Originating-IP: [10.35.28.120] X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprPLMWRmVeSWpSXmKPExsUyM2K7sa4l788Ig9u3WS3mnG9hsTi2QMxi y/ksi8fXrzM7sHhsWXmTyWP3gs9MHp83yXms37KVKYAlissmJTUnsyy1SN8ugSvjw5fPLAWn pCs2bdNtYJwj2sXIySEhYCLR+nABSxcjF4eQwHpGiamzvrNCONsZJY49+csEUsUmoCHxcloH I4gtImAs8WplJ1icWaBA4unEf0ANHBzCAvYSD7Y5gpgsAqoSuz8rglTwCrhKbL2/gRlil5zE +eM/mSHighInZz5hgZgiIXHwxQuwuJCAssTcD9OYIOoVJL7N7GaawMg3C0nLLCQtCxiZVjGK FqcWJ+WmGxnrpRZlJhcX5+fp5aWWbGIEBtvBLb9VdzBefuN4iFGAg1GJh/fBvh8RQqyJZcWV uYcYJTiYlUR4e7cDhXhTEiurUovy44tKc1KLDzFKc7AoifM67rsQISSQnliSmp2aWpBaBJNl 4uCUamA0cdg/t/HiGybTlIvxKcoGzgK1Ncmm0YXd6Yl7G5cZGj1eeyfMZtFtpcO+0w95Bs48 q7858/vr9TsSZVas/39c5yDzr6d/hR4t0nu9s+jnJG7J9rauMqkjJa/k+6RvvVQQW2V1qfPK s5V8tiz3v/91Otx1dueFLULa/ql3VJbbvamO727JqYpTYinOSDTUYi4qTgQA7/5pqDICAAA= Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Jon Paul Maloy commit d25a01257e422a4bdeb426f69529d57c73b235fe upstream When the TIPC module is unloaded, we have identified a race condition that allows a node reference counter to go to zero and the node instance being freed before the node timer is finished with accessing it. This leads to occasional crashes, especially in multi-namespace environments. The scenario goes as follows: CPU0:(node_stop) CPU1:(node_timeout) // ref == 2 1: if(!mod_timer()) 2: if (del_timer()) 3: tipc_node_put() // ref -> 1 4: tipc_node_put() // ref -> 0 5: kfree_rcu(node); 6: tipc_node_get(node) 7: // BOOM! We now clean up this functionality as follows: 1) We remove the node pointer from the node lookup table before we attempt deactivating the timer. This way, we reduce the risk that tipc_node_find() may obtain a valid pointer to an instance marked for deletion; a harmless but undesirable situation. 2) We use del_timer_sync() instead of del_timer() to safely deactivate the node timer without any risk that it might be reactivated by the timeout handler. There is no risk of deadlock here, since the two functions never touch the same spinlocks. 3: We remove a pointless tipc_node_get() + tipc_node_put() from the timeout handler. Reported-by: Zhijiang Hu Acked-by: Ying Xue Signed-off-by: Jon Maloy Signed-off-by: David S. Miller --- net/tipc/node.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/net/tipc/node.c b/net/tipc/node.c index 3926b56..d468aad 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -102,9 +102,10 @@ static unsigned int tipc_hashfn(u32 addr) static void tipc_node_kref_release(struct kref *kref) { - struct tipc_node *node = container_of(kref, struct tipc_node, kref); + struct tipc_node *n = container_of(kref, struct tipc_node, kref); - tipc_node_delete(node); + kfree(n->bc_entry.link); + kfree_rcu(n, rcu); } void tipc_node_put(struct tipc_node *node) @@ -216,21 +217,20 @@ static void tipc_node_delete(struct tipc_node *node) { list_del_rcu(&node->list); hlist_del_rcu(&node->hash); - kfree(node->bc_entry.link); - kfree_rcu(node, rcu); + tipc_node_put(node); + + del_timer_sync(&node->timer); + tipc_node_put(node); } void tipc_node_stop(struct net *net) { - struct tipc_net *tn = net_generic(net, tipc_net_id); + struct tipc_net *tn = tipc_net(net); struct tipc_node *node, *t_node; spin_lock_bh(&tn->node_list_lock); - list_for_each_entry_safe(node, t_node, &tn->node_list, list) { - if (del_timer(&node->timer)) - tipc_node_put(node); - tipc_node_put(node); - } + list_for_each_entry_safe(node, t_node, &tn->node_list, list) + tipc_node_delete(node); spin_unlock_bh(&tn->node_list_lock); } @@ -313,9 +313,7 @@ static void tipc_node_timeout(unsigned long data) if (rc & TIPC_LINK_DOWN_EVT) tipc_node_link_down(n, bearer_id, false); } - if (!mod_timer(&n->timer, jiffies + n->keepalive_intv)) - tipc_node_get(n); - tipc_node_put(n); + mod_timer(&n->timer, jiffies + n->keepalive_intv); } /**