From b55c59892e1f3b6c7d4b9ccffb4263e1486fb990 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 6 Jul 2011 19:58:23 -0400 Subject: 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 Tested-by: Ben Greear Signed-off-by: Trond Myklebust Cc: stable@kernel.org --- net/sunrpc/sched.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) (limited to 'net') diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c index a27406b1654..4814e246a87 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. -- cgit v1.2.3 From ec0dd267bf7d08cb30e321e45a75fd40edd7e528 Mon Sep 17 00:00:00 2001 From: Ben Greear Date: Tue, 12 Jul 2011 10:27:55 -0700 Subject: SUNRPC: Fix use of static variable in rpcb_getport_async Because struct rpcbind_args *map was declared static, if two threads entered this method at the same time, the values assigned to map could be sent two two differen tasks. This could cause all sorts of problems, include use-after-free and double-free of memory. Fix this by removing the static declaration so that the map pointer is on the stack. Signed-off-by: Ben Greear Cc: stable@kernel.org Signed-off-by: Trond Myklebust --- net/sunrpc/rpcb_clnt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c index 9a80a922c52..e45d2fbbe5a 100644 --- a/net/sunrpc/rpcb_clnt.c +++ b/net/sunrpc/rpcb_clnt.c @@ -597,7 +597,7 @@ void rpcb_getport_async(struct rpc_task *task) u32 bind_version; struct rpc_xprt *xprt; struct rpc_clnt *rpcb_clnt; - static struct rpcbind_args *map; + struct rpcbind_args *map; struct rpc_task *child; struct sockaddr_storage addr; struct sockaddr *sap = (struct sockaddr *)&addr; -- cgit v1.2.3