[tipc-discussion] [PATCH net v2 1/1] tipc: fix crash during node removal

[Jon Maloy]

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
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!

In this commit, we reverse the condition for counter incrementation/
decrementation in the timer function, so we get two mutually exclusive
tipc_node_put() calls.

We get the following new scenario:

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_put() // not called

As a consequence of this, we can now remove the second decrementation
in the timout function, since we never do any incrementation in the
first place.

Reported-by: Jason Huzhijiang 
Signed-off-by: Jon Maloy 
---
 net/tipc/node.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 9d7a16f..50285de 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -530,9 +530,8 @@ 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(>timer, jiffies + n->keepalive_intv))
-   tipc_node_get(n);
-   tipc_node_put(n);
+   if (mod_timer(>timer, jiffies + n->keepalive_intv))
+   tipc_node_put(n);
 }
 
 /**
-- 
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=/4140
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

[tipc-discussion] [PATCH net v2 1/1] tipc: fix crash during node removal

[Jon Maloy]

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
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!

In this commit, we reverse the condition for counter incrementation/
decrementation in the timer function, so we get two mutually exclusive
tipc_node_put() calls.

We get the following new scenario:

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_put() // not called

As a consequence of this, we can now remove the second decrementation
in the timout function, since we never do any incrementation in the
first place.

Reported-by: Jason Huzhijiang 
Signed-off-by: Jon Maloy 
---
 net/tipc/node.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 9d7a16f..50285de 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -530,9 +530,8 @@ 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(>timer, jiffies + n->keepalive_intv))
-   tipc_node_get(n);
-   tipc_node_put(n);
+   if (mod_timer(>timer, jiffies + n->keepalive_intv))
+   tipc_node_put(n);
 }
 
 /**
-- 
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=/4140
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion