Skip to content

Commit

Permalink
NLM: Fix a circular lock dependency in lockd
Browse files Browse the repository at this point in the history
The problem is that the garbage collector for the 'host' structures
nlm_gc_hosts(), holds nlm_host_mutex while calling down to
nlmsvc_mark_resources, which, eventually takes the file->f_mutex.

We cannot therefore call nlmsvc_lookup_host() from within
nlmsvc_create_block, since the caller will already hold file->f_mutex, so
the attempt to grab nlm_host_mutex may deadlock.

Fix the problem by calling nlmsvc_lookup_host() outside the file->f_mutex.

Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Trond Myklebust authored and Linus Torvalds committed Sep 26, 2007
1 parent f6a592e commit 255129d
Showing 1 changed file with 18 additions and 11 deletions.
29 changes: 18 additions & 11 deletions fs/lockd/svclock.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,19 +171,14 @@ nlmsvc_find_block(struct nlm_cookie *cookie)
* GRANTED_RES message by cookie, without having to rely on the client's IP
* address. --okir
*/
static inline struct nlm_block *
nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_file *file,
struct nlm_lock *lock, struct nlm_cookie *cookie)
static struct nlm_block *
nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_host *host,
struct nlm_file *file, struct nlm_lock *lock,
struct nlm_cookie *cookie)
{
struct nlm_block *block;
struct nlm_host *host;
struct nlm_rqst *call = NULL;

/* Create host handle for callback */
host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
if (host == NULL)
return NULL;

call = nlm_alloc_call(host);
if (call == NULL)
return NULL;
Expand Down Expand Up @@ -366,6 +361,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;
__be32 ret;

Expand All @@ -377,6 +373,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, lock->caller, lock->len);
if (host == NULL)
return nlm_lck_denied_nolocks;

/* Lock file against concurrent access */
mutex_lock(&file->f_mutex);
Expand All @@ -385,7 +385,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);
block = nlmsvc_create_block(rqstp, nlm_get_host(host), file,
lock, cookie);
ret = nlm_lck_denied_nolocks;
if (block == NULL)
goto out;
Expand Down Expand Up @@ -449,6 +450,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
out:
mutex_unlock(&file->f_mutex);
nlmsvc_release_block(block);
nlm_release_host(host);
dprintk("lockd: nlmsvc_lock returned %u\n", ret);
return ret;
}
Expand Down Expand Up @@ -477,10 +479,15 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,

if (block == NULL) {
struct file_lock *conf = kzalloc(sizeof(*conf), GFP_KERNEL);
struct nlm_host *host;

if (conf == NULL)
return nlm_granted;
block = nlmsvc_create_block(rqstp, file, lock, cookie);
/* Create host handle for callback */
host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
if (host == NULL)
return nlm_lck_denied_nolocks;
block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
if (block == NULL) {
kfree(conf);
return nlm_granted;
Expand Down

0 comments on commit 255129d

Please sign in to comment.