SUNRPC: Fix a race between work-queue and rpc_killall_tasks

[ upstream commit b55c59892e1f3b6c7d4b9ccffb4263e1486fb990 ]

Since rpc_killall_tasks may modify the rpc_task's tk_action field
without any locking, we need to be careful when dereferencing it.

Reported-by: Ben Greear <greearb@candelatech.com>
Tested-by: Ben Greear <greearb@candelatech.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: stable@kernel.org
Signed-off-by: Andi Kleen <ak@linux.intel.com>

diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index c55f753b09c060acf8c1b7ba940dcbf319c14273..f54059b5f3206bd51ea51f238f93704b96421ee4 100644 (file)
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -623,30 +623,25 @@ static void __rpc_execute(struct rpc_task *task)
        BUG_ON(RPC_IS_QUEUED(task));
 
        for (;;) {
+               void (*do_action)(struct rpc_task *);
 
                /*
-                * Execute any pending callback.
+                * Execute any pending callback first.
                 */
-               if (task->tk_callback) {
-                       void (*save_callback)(struct rpc_task *);
-
-                       /*
-                        * We set tk_callback to NULL before calling it,
-                        * in case it sets the tk_callback field itself:
-                        */
-                       save_callback = task->tk_callback;
-                       task->tk_callback = NULL;
-                       save_callback(task);
-               } else {
+               do_action = task->tk_callback;
+               task->tk_callback = NULL;
+               if (do_action == NULL) {
                        /*
                         * Perform the next FSM step.
-                        * tk_action may be NULL when the task has been killed
-                        * by someone else.
+                        * tk_action may be NULL if the task has been killed.
+                        * In particular, note that rpc_killall_tasks may
+                        * do this at any time, so beware when dereferencing.
                         */
-                       if (task->tk_action == NULL)
+                       do_action = task->tk_action;
+                       if (do_action == NULL)
                                break;
-                       task->tk_action(task);
                }
+               do_action(task);
 
                /*
                 * Lockless check for whether task is sleeping or not.