SUNRPC: Fix a race between work-queue and rpc_killall_tasks

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

1 files changed, 11 insertions, 16 deletions

diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index a27406b1654f..4814e246a874 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -616,30 +616,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.