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
