Attachment 311206 Details for Bug 453094 – proposed patch

[patch] proposed patch

bz-453094-nlm-don-t-call-nlmsv (text/plain), 3.25 KB, created by Jeff Layton on 2008-07-07 21:55:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Layton

Created: 2008-07-07 21:55:18 UTC

Size: 3.25 KB

patch

obsolete

>BZ#453094: NLM: don't call nlmsvc_lookup_host with the f_sema held
>
>From: Jeff Layton <jlayton@redhat.com>
>
>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 move the
>nlmsvc_lookup_host call outside of the f_sema.
>---
>
> fs/lockd/svclock.c |   27 +++++++++++++++++----------
> 1 files changed, 17 insertions(+), 10 deletions(-)
>
>
>diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
>index f272f2c..91af39d 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;
>@@ -390,6 +385,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
> 			struct nlm_lock *lock, int wait, struct nlm_cookie *cookie)
> {
> 	struct nlm_block	*block = NULL;
>+	struct nlm_host		*host;
> 	int			error;
> 	u32			ret;
> 
>@@ -401,6 +397,10 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
> 				(long long)lock->fl.fl_end,
> 				wait);
> 
>+	/* Create host handle for callback */
>+	host = nlmsvc_lookup_host(rqstp);
>+	if (host == NULL)
>+		return nlm_lck_denied_nolocks;
> 
> 	/* Lock file against concurrent access */
> 	down(&file->f_sema);
>@@ -409,7 +409,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;
>@@ -473,6 +474,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
> out:
> 	up(&file->f_sema);
> 	nlmsvc_release_block(block);
>+	nlm_release_host(host);
> 	dprintk("lockd: nlmsvc_lock returned %u\n", ret);
> 	return ret;
> }
>@@ -486,6 +488,7 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
> 		struct nlm_cookie *cookie)
> {
> 	struct nlm_block 	*block = NULL;
>+	struct nlm_host		*host;
> 	int			error;
> 	__be32			ret;
> 
>@@ -500,7 +503,11 @@ 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);
>+		/* Create host handle for callback */
>+		host = nlmsvc_lookup_host(rqstp);
>+		if (host == NULL)
>+			return nlm_lck_denied_nolocks;
>+		block = nlmsvc_create_block(rqstp, host, file, lock, cookie, 1);
> 		if (block == NULL)
> 			return nlm_granted;
> 	}

Actions: View | Diff

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