Attachment 311599 Details for Bug 453094 – proposed patch 2

[patch] proposed patch 2

0001-BZ-453094-NLM-don-t-call-nlmsvc_lookup_host-with-t.patch (text/plain), 6.21 KB, created by Jeff Layton on 2008-07-11 16:10:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Layton

Created: 2008-07-11 16:10:26 UTC

Size: 6.21 KB

patch

obsolete

>From 1f251e578faaae429bc72b45b82f25c8bd38eb75 Mon Sep 17 00:00:00 2001
>From: Jeff Layton <jlayton@redhat.com>
>Date: Fri, 11 Jul 2008 12:07:58 -0400
>Subject: [PATCH] BZ#453094: NLM: don't call nlmsvc_lookup_host with the f_sema held
>
>There is a possible deadlock in the NLM code where lockd can try to
>take the f_sema for an nlm_file that it already holds. The call
>chain is something like:
>
>nlmsvc_lock (take the f_sema)
>  nlmsvc_create_block
>     nlmsvc_lookup_host
>         nlm_gc_hosts
>            nlmsvc_traverse_files (try to take the f_sema)
>
>...obviously, this will deadlock. The fix is to just use the host
>pointer that we get from nlmsvc_retrieve_args and pass it down
>nlmsvc_create_block. We're doing a duplicate call for each lock
>and testlock anyway so this is more efficient and avoids the
>deadlock.
>---
> fs/lockd/svc4proc.c         |    4 ++--
> fs/lockd/svclock.c          |   25 +++++++++++--------------
> fs/lockd/svcproc.c          |    6 +++---
> include/linux/lockd/lockd.h |    7 ++++---
> 4 files changed, 20 insertions(+), 22 deletions(-)
>
>diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
>index 2eb0a61..236b211 100644
>--- a/fs/lockd/svc4proc.c
>+++ b/fs/lockd/svc4proc.c
>@@ -103,7 +103,7 @@ nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_args *argp,
> 			rpc_drop_reply :rpc_success;
> 
> 	/* Now check for conflicting locks */
>-	resp->status = nlmsvc_testlock(rqstp, file, &argp->lock,
>+	resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock,
> 				       &resp->lock, &resp->cookie);
> 	if (resp->status == nlm_drop_reply)
> 		return rpc_drop_reply;
>@@ -149,7 +149,7 @@ nlm4svc_proc_lock(struct svc_rqst *rqstp, struct nlm_args *argp,
> #endif
> 
> 	/* Now try to lock the file */
>-	resp->status = nlmsvc_lock(rqstp, file, &argp->lock,
>+	resp->status = nlmsvc_lock(rqstp, file, host, &argp->lock,
> 					argp->block, &argp->cookie);
> 	if (resp->status == nlm_drop_reply)
> 		return rpc_drop_reply;
>diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
>index f272f2c..4e2fb11 100644
>--- a/fs/lockd/svclock.c
>+++ b/fs/lockd/svclock.c
>@@ -171,18 +171,13 @@ nlmsvc_find_block(struct nlm_cookie *cookie,  struct sockaddr_in *sin)
>  * logging industries.
>  */
> static inline struct nlm_block *
>-nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_file *file,
>-		struct nlm_lock *lock, struct nlm_cookie *cookie, int conf)
>+nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_host *host,
>+		struct nlm_file *file, struct nlm_lock *lock,
>+		struct nlm_cookie *cookie, int conf)
> {
> 	struct nlm_block	*block;
>-	struct nlm_host		*host;
> 	struct nlm_rqst		*call = NULL;
> 
>-	/* Create host handle for callback */
>-	host = nlmsvc_lookup_host(rqstp);
>-	if (host == NULL)
>-		return NULL;
>-
> 	call = nlm_alloc_call(host);
> 	if (call == NULL)
> 		return NULL;
>@@ -387,7 +382,8 @@ nlmsvc_defer_lock_rqst(struct svc_rqst *rqstp, struct nlm_block *block)
>  */
> u32
> nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
>-			struct nlm_lock *lock, int wait, struct nlm_cookie *cookie)
>+		struct nlm_host *host, struct nlm_lock *lock,
>+		int wait, struct nlm_cookie *cookie)
> {
> 	struct nlm_block	*block = NULL;
> 	int			error;
>@@ -401,7 +397,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
> 				(long long)lock->fl.fl_end,
> 				wait);
> 
>-
> 	/* Lock file against concurrent access */
> 	down(&file->f_sema);
> 	/* Get existing block (in case client is busy-waiting)
>@@ -409,7 +404,8 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
> 	 */
> 	block = nlmsvc_lookup_block(file, lock);
> 	if (block == NULL) {
>-		block = nlmsvc_create_block(rqstp, file, lock, cookie, 0);
>+		block = nlmsvc_create_block(rqstp, nlm_get_host(host), file,
>+				lock, cookie, 0);
> 		ret = nlm_lck_denied_nolocks;
> 		if (block == NULL)
> 			goto out;
>@@ -482,8 +478,8 @@ out:
>  */
> u32
> nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
>-		struct nlm_lock *lock, struct nlm_lock *conflock,
>-		struct nlm_cookie *cookie)
>+		struct nlm_host *host, struct nlm_lock *lock,
>+		struct nlm_lock *conflock, struct nlm_cookie *cookie)
> {
> 	struct nlm_block 	*block = NULL;
> 	int			error;
>@@ -500,7 +496,8 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
> 	block = nlmsvc_lookup_block(file, lock);
> 
> 	if (block == NULL) {
>-		block = nlmsvc_create_block(rqstp, file, lock, cookie, 1);
>+		block = nlmsvc_create_block(rqstp, nlm_get_host(host), file,
>+					    lock, cookie, 1);
> 		if (block == NULL)
> 			return nlm_granted;
> 	}
>diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
>index dc6e5f7..46cf060 100644
>--- a/fs/lockd/svcproc.c
>+++ b/fs/lockd/svcproc.c
>@@ -132,8 +132,8 @@ nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_args *argp,
> 			rpc_drop_reply :rpc_success;
> 
> 	/* Now check for conflicting locks */
>-	resp->status = cast_status(nlmsvc_testlock(rqstp, file, &argp->lock,
>-				   &resp->lock, &resp->cookie));
>+	resp->status = cast_status(nlmsvc_testlock(rqstp, file, host,
>+				   &argp->lock, &resp->lock, &resp->cookie));
> 	if (resp->status == nlm_drop_reply)
> 		return rpc_drop_reply;
> 
>@@ -179,7 +179,7 @@ nlmsvc_proc_lock(struct svc_rqst *rqstp, struct nlm_args *argp,
> #endif
> 
> 	/* Now try to lock the file */
>-	resp->status = cast_status(nlmsvc_lock(rqstp, file, &argp->lock,
>+	resp->status = cast_status(nlmsvc_lock(rqstp, file, host, &argp->lock,
> 					       argp->block, &argp->cookie));
> 	if (resp->status == nlm_drop_reply)
> 		return rpc_drop_reply;
>diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
>index 0f92d53..eeaf356 100644
>--- a/include/linux/lockd/lockd.h
>+++ b/include/linux/lockd/lockd.h
>@@ -186,11 +186,12 @@ extern struct nlm_host *nlm_find_client(void);
>  * Server-side lock handling
>  */
> u32		  nlmsvc_lock(struct svc_rqst *, struct nlm_file *,
>-					struct nlm_lock *, int, struct nlm_cookie *);
>+				struct nlm_host *, struct nlm_lock *, int,
>+				struct nlm_cookie *);
> u32		  nlmsvc_unlock(struct nlm_file *, struct nlm_lock *);
> u32		  nlmsvc_testlock(struct svc_rqst *, struct nlm_file *,
>-				  struct nlm_lock *, struct nlm_lock *,
>-				  struct nlm_cookie *);
>+				  struct nlm_host *, struct nlm_lock *,
>+				  struct nlm_lock *, struct nlm_cookie *);
> u32		  nlmsvc_cancel_blocked(struct nlm_file *, struct nlm_lock *);
> unsigned long	  nlmsvc_retry_blocked(void);
> void		  nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *,
>-- 
>1.5.5.1
>

Actions: View | Diff

Attachments on bug 453094: 311104 | 311206 | 311582 | 311599 | 311951