Hi Jason, Not sure you are on the tipc-discussion list, so I sent out my patches a second time. I think those should solve the problems you have identified. Did I get your name right?
Regards ///jon -------- Forwarded Message -------- Subject: [PATCH net-next 1/1] tipc: eliminate risk of finding to-be-deleted node instance Date: Tue, 16 Feb 2016 08:30:35 -0500 From: Jon Maloy <[email protected]> To: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected] CC: [email protected] Although we have never seen it happen, we have identified the following problematic scenario when nodes are stopped and deleted: CPU0: CPU1: tipc_node_xxx() //ref == 1 tipc_node_put() //ref -> 0 tipc_node_find() // node still in table tipc_node_delete() list_del_rcu(n. list) tipc_node_get() //ref -> 1, bad kfree_rcu() tipc_node_put() //ref to 0 again. kfree_rcu() // BOOM! We now clean this up by making two changes: 1: Ensure that the node is removed from the node table before we try deleting the timer. Although a cosmetic change, this is a more logical order. Whichever comes last of the functions node_delete() and node_timeout() to put kref to zero will trig the actual freeing of memory. 2: We introduce use of the conditional kref_get_if_not_zero() instead of kref_get() in tipc_node_find(). This eliminates any risk of post-mortem access. Reported-by: Jason Huzhijiang <[email protected]> Signed-off-by: Jon Maloy <[email protected]> --- net/tipc/node.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/net/tipc/node.c b/net/tipc/node.c index e1c9e39..67f4fe5 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -150,7 +150,6 @@ static void __tipc_node_link_down(struct tipc_node *n, int *bearer_id, static void tipc_node_link_down(struct tipc_node *n, int bearer_id, bool delete); static void node_lost_contact(struct tipc_node *n, struct sk_buff_head *inputq); -static void tipc_node_delete(struct tipc_node *node); static void tipc_node_timeout(unsigned long data); static void tipc_node_fsm_evt(struct tipc_node *n, int evt); static struct tipc_node *tipc_node_find(struct net *net, u32 addr); @@ -227,7 +226,8 @@ static void tipc_node_kref_release(struct kref *kref) { struct tipc_node *node = container_of(kref, struct tipc_node, kref); - tipc_node_delete(node); + kfree(node->bc_entry.link); + kfree_rcu(node, rcu); } static void tipc_node_put(struct tipc_node *node) @@ -245,20 +245,21 @@ static void tipc_node_get(struct tipc_node *node) */ static struct tipc_node *tipc_node_find(struct net *net, u32 addr) { - struct tipc_net *tn = net_generic(net, tipc_net_id); + struct tipc_net *tn = tipc_net(net); struct tipc_node *node; + unsigned int thash = tipc_hashfn(addr); if (unlikely(!in_own_cluster_exact(net, addr))) return NULL; rcu_read_lock(); - hlist_for_each_entry_rcu(node, &tn->node_htable[tipc_hashfn(addr)], - hash) { - if (node->addr == addr) { - tipc_node_get(node); - rcu_read_unlock(); - return node; - } + hlist_for_each_entry_rcu(node, &tn->node_htable[thash], hash) { + if (node->addr != addr) + continue; + if (!kref_get_unless_zero(&node->kref)) + break; + rcu_read_unlock(); + return node; } rcu_read_unlock(); return NULL; @@ -395,21 +396,19 @@ 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); + if (del_timer(&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); } -- 1.9.1 ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 _______________________________________________ tipc-discussion mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tipc-discussion
