* Re: [PATCH] SUNRPC: Invalid BUG_ON() popping in rpc_release_task()
2007-01-21 11:53 [PATCH] SUNRPC: Invalid BUG_ON() popping in rpc_release_task() Steve Dickson
@ 2007-01-21 17:25 ` Trond Myklebust
2007-01-21 19:59 ` Steve Dickson
0 siblings, 1 reply; 3+ messages in thread
From: Trond Myklebust @ 2007-01-21 17:25 UTC (permalink / raw)
To: Steve Dickson; +Cc: nfs
[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]
On Sun, 2007-01-21 at 06:53 -0500, Steve Dickson wrote:
> Over the weekend, while doing some Secure NFS testing, I found
> that the BUG_ON() in rpc_release_task() would pop when
> the Kerberos server was not set up correctly (which I know is a
> very common problem when people start using this feature).
>
> The code path looks like:
> rpc_call_sync() calls rpc_call_setup() which calls rpcauth_bindcred()
> which fails (due to a krb5 problem) and sets tk_status to -EACCES.
>
> Back in rpc_call_sync(), since (tk_status != 0) rpc_release_task()
> is called. Unfortunately, since this task was never active,
> (meaning rpc_execute() and rpc_set_active() were never called)
> the tk_magic id was never set and the global task list was
> never set up. So when rpc_release_task() is call, the world
> come tumbling down...
>
> So the attached patch ensures the RPC_TASK_ACTIVE run state bit
> is set before calling the BUG_ON() and cleaning up the task
> list.
>
>
> steved.
I've already posted the attach patch to fix this issue.
Cheers,
Trond
[-- Attachment #2: linux-2.6.20-001-fix_rpc_call_sync.dif --]
[-- Type: message/rfc822, Size: 2592 bytes --]
From: Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: No Subject
Date: Tue, 16 Jan 2007 07:23:06 -0500
Message-ID: <1169400333.5764.3.camel@lade.trondhjem.org>
We shouldn't be calling rpc_release_task() for tasks that are not active.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
include/linux/sunrpc/sched.h | 1 -
net/sunrpc/clnt.c | 8 +++-----
net/sunrpc/sched.c | 3 ++-
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index 97c7616..8b6ce60 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -250,7 +250,6 @@ void rpc_init_task(struct rpc_task *tas
int flags, const struct rpc_call_ops *ops,
void *data);
void rpc_put_task(struct rpc_task *);
-void rpc_release_task(struct rpc_task *);
void rpc_exit_task(struct rpc_task *);
void rpc_release_calldata(const struct rpc_call_ops *, void *);
void rpc_killall_tasks(struct rpc_clnt *);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index aba528b..16c9fbc 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -490,16 +490,14 @@ int rpc_call_sync(struct rpc_clnt *clnt,
/* Set up the call info struct and execute the task */
status = task->tk_status;
- if (status != 0) {
- rpc_release_task(task);
+ if (status != 0)
goto out;
- }
atomic_inc(&task->tk_count);
status = rpc_execute(task);
if (status == 0)
status = task->tk_status;
- rpc_put_task(task);
out:
+ rpc_put_task(task);
rpc_restore_sigmask(&oldset);
return status;
}
@@ -537,7 +535,7 @@ rpc_call_async(struct rpc_clnt *clnt, st
if (status == 0)
rpc_execute(task);
else
- rpc_release_task(task);
+ rpc_put_task(task);
rpc_restore_sigmask(&oldset);
return status;
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 79bc4cd..fc083f0 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -42,6 +42,7 @@ static mempool_t *rpc_buffer_mempool __r
static void __rpc_default_timer(struct rpc_task *task);
static void rpciod_killall(void);
static void rpc_async_schedule(struct work_struct *);
+static void rpc_release_task(struct rpc_task *task);
/*
* RPC tasks sit here while waiting for conditions to improve.
@@ -896,7 +897,7 @@ void rpc_put_task(struct rpc_task *task)
}
EXPORT_SYMBOL(rpc_put_task);
-void rpc_release_task(struct rpc_task *task)
+static void rpc_release_task(struct rpc_task *task)
{
#ifdef RPC_DEBUG
BUG_ON(task->tk_magic != RPC_TASK_MAGIC_ID);
[-- Attachment #3: Type: text/plain, Size: 347 bytes --]
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
[-- Attachment #4: Type: text/plain, Size: 140 bytes --]
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
^ permalink raw reply related [flat|nested] 3+ messages in thread