[PATCH net 0/2] tipc: corrections related to tasklet job mechanism

[PATCH net 0/2] tipc: corrections related to tasklet job mechanism

[Jon Maloy]

These commits correct two bugs related to tipc' service for launching
functions for asynchronous execution in a separate tasklet.

Jon Maloy (1):
  tipc: correct the order of stopping services at rmmod

Ying Xue (1):
  tipc: protect handler_enabled variable with qitem_lock spin lock

 net/tipc/core.c    |    7 ++++---
 net/tipc/handler.c |   11 ++++++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

-- 
1.7.9.5


------------------------------------------------------------------------------
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk

[PATCH net 1/2] tipc: correct the order of stopping services at rmmod

[Jon Maloy]

The 'signal handler' service in TIPC is a mechanism that makes it
possible to postpone execution of functions, by launcing them into
a job queue for execution in a separate tasklet, independent of
the launching execution thread.

When we do rmmod on the tipc module, this service is stopped after
the network service. At the same time, the stopping of the network
service may itself launch jobs for execution, with the risk that these
functions may be scheduled for execution after the data structures
meant to be accessed by the job have already been deleted. We have
seen this happen, most often resulting in an oops.

This commit ensures that the signal handler is the very first to be
stopped when TIPC is shut down, so there are no surprises during
the cleanup of the other services.

Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/core.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/tipc/core.c b/net/tipc/core.c
index fd4eeea..c6d3f75 100644
--- a/net/tipc/core.c
+++ b/net/tipc/core.c
@@ -113,7 +113,6 @@ err:
 static void tipc_core_stop(void)
 {
 	tipc_netlink_stop();
-	tipc_handler_stop();
 	tipc_cfg_stop();
 	tipc_subscr_stop();
 	tipc_nametbl_stop();
@@ -146,9 +145,10 @@ static int tipc_core_start(void)
 		res = tipc_subscr_start();
 	if (!res)
 		res = tipc_cfg_init();
-	if (res)
+	if (res) {
+		tipc_handler_stop();
 		tipc_core_stop();
-
+	}
 	return res;
 }
 
@@ -178,6 +178,7 @@ static int __init tipc_init(void)
 
 static void __exit tipc_exit(void)
 {
+	tipc_handler_stop();
 	tipc_core_stop_net();
 	tipc_core_stop();
 	pr_info("Deactivated\n");
-- 
1.7.9.5


------------------------------------------------------------------------------
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk

[PATCH net 2/2] tipc: protect handler_enabled variable with qitem_lock spin lock

[Jon Maloy]

From: Ying Xue <ying.xue@windriver.com>

'handler_enabled' is a global flag indicating whether the TIPC
signal handling service is enabled or not. The lack of lock
protection for this flag incurs a risk for contention, so that
a tipc_k_signal() call might queue a signal handler to a destroyed
signal queue, with unpredictable results. To correct this, we let
the already existing 'qitem_lock' protect the flag, as it already
does with the queue itself. This way, we ensure that the flag
always is consistent across all cores.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/handler.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/tipc/handler.c b/net/tipc/handler.c
index b36f0fc..e4bc8a2 100644
--- a/net/tipc/handler.c
+++ b/net/tipc/handler.c
@@ -56,12 +56,13 @@ unsigned int tipc_k_signal(Handler routine, unsigned long argument)
 {
 	struct queue_item *item;
 
+	spin_lock_bh(&qitem_lock);
 	if (!handler_enabled) {
 		pr_err("Signal request ignored by handler\n");
+		spin_unlock_bh(&qitem_lock);
 		return -ENOPROTOOPT;
 	}
 
-	spin_lock_bh(&qitem_lock);
 	item = kmem_cache_alloc(tipc_queue_item_cache, GFP_ATOMIC);
 	if (!item) {
 		pr_err("Signal queue out of memory\n");
@@ -112,10 +113,14 @@ void tipc_handler_stop(void)
 	struct list_head *l, *n;
 	struct queue_item *item;
 
-	if (!handler_enabled)
+	spin_lock_bh(&qitem_lock);
+	if (!handler_enabled) {
+		spin_unlock_bh(&qitem_lock);
 		return;
-
+	}
 	handler_enabled = 0;
+	spin_unlock_bh(&qitem_lock);
+
 	tasklet_kill(&tipc_tasklet);
 
 	spin_lock_bh(&qitem_lock);
-- 
1.7.9.5


------------------------------------------------------------------------------
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk

Re: [PATCH net 0/2] tipc: corrections related to tasklet job mechanism

[David Miller]

From: Jon Maloy <jon.maloy@ericsson.com>
Date: Mon,  9 Dec 2013 22:54:45 -0800

> These commits correct two bugs related to tipc' service for launching
> functions for asynchronous execution in a separate tasklet.

Series applied, thanks.