This patch fixes a race in async lock request handling between adding
the relevant struct nlm_block to nlm_blocked list after the request was
sent by vfs_lock_file() and nlmsvc_grant_deferred() does a lookup of the
nlm_block in the nlm_blocked list. It could be that the async request is
completed before the nlm_block was added to the list. This would end
in a -ENOENT and a kernel log message of "lockd: grant for unknown
block".

To solve this issue we add the nlm_block before the vfs_lock_file() call
to be sure it has been added when a possible nlmsvc_grant_deferred() is
called. If the vfs_lock_file() results in an case when it wouldn't be
added to nlm_blocked list, the nlm_block struct will be removed from
this list again.

Signed-off-by: Alexander Aring <aahri...@redhat.com>
---
 fs/lockd/svclock.c          | 80 +++++++++++++++++++++++++++----------
 include/linux/lockd/lockd.h |  1 +
 2 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 28abec5c451d..62ef27a69a9e 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -297,6 +297,8 @@ static void nlmsvc_free_block(struct kref *kref)
 
        dprintk("lockd: freeing block %p...\n", block);
 
+       WARN_ON_ONCE(block->b_flags & B_PENDING_CALLBACK);
+
        /* Remove block from file's list of blocks */
        list_del_init(&block->b_flist);
        mutex_unlock(&file->f_mutex);
@@ -543,6 +545,12 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
                goto out;
        }
 
+       if (block->b_flags & B_PENDING_CALLBACK)
+               goto pending_request;
+
+       /* Append to list of blocked */
+       nlmsvc_insert_block(block, NLM_NEVER);
+
        if (!wait)
                lock->fl.fl_flags &= ~FL_SLEEP;
        mode = lock_to_openmode(&lock->fl);
@@ -552,9 +560,13 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
        dprintk("lockd: vfs_lock_file returned %d\n", error);
        switch (error) {
                case 0:
+                       nlmsvc_remove_block(block);
                        ret = nlm_granted;
                        goto out;
                case -EAGAIN:
+                       if (!wait)
+                               nlmsvc_remove_block(block);
+pending_request:
                        /*
                         * If this is a blocking request for an
                         * already pending lock request then we need
@@ -565,6 +577,8 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
                        ret = async_block ? nlm_lck_blocked : nlm_lck_denied;
                        goto out;
                case FILE_LOCK_DEFERRED:
+                       block->b_flags |= B_PENDING_CALLBACK;
+
                        if (wait)
                                break;
                        /* Filesystem lock operation is in progress
@@ -572,17 +586,16 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
                        ret = nlmsvc_defer_lock_rqst(rqstp, block);
                        goto out;
                case -EDEADLK:
+                       nlmsvc_remove_block(block);
                        ret = nlm_deadlock;
                        goto out;
                default:                        /* includes ENOLCK */
+                       nlmsvc_remove_block(block);
                        ret = nlm_lck_denied_nolocks;
                        goto out;
        }
 
        ret = nlm_lck_blocked;
-
-       /* Append to list of blocked */
-       nlmsvc_insert_block(block, NLM_NEVER);
 out:
        mutex_unlock(&file->f_mutex);
        nlmsvc_release_block(block);
@@ -739,34 +752,59 @@ nlmsvc_update_deferred_block(struct nlm_block *block, int 
result)
                block->b_flags |= B_TIMED_OUT;
 }
 
+static int __nlmsvc_grant_deferred(struct nlm_block *block,
+                                  struct file_lock *fl,
+                                  int result)
+{
+       int rc = 0;
+
+       dprintk("lockd: nlmsvc_notify_blocked block %p flags %d\n",
+                                       block, block->b_flags);
+       if (block->b_flags & B_QUEUED) {
+               if (block->b_flags & B_TIMED_OUT) {
+                       rc = -ENOLCK;
+                       goto out;
+               }
+               nlmsvc_update_deferred_block(block, result);
+       } else if (result == 0)
+               block->b_granted = 1;
+
+       nlmsvc_insert_block_locked(block, 0);
+       svc_wake_up(block->b_daemon);
+out:
+       return rc;
+}
+
 static int nlmsvc_grant_deferred(struct file_lock *fl, int result)
 {
-       struct nlm_block *block;
-       int rc = -ENOENT;
+       struct nlm_block *block = NULL;
+       int rc;
 
        spin_lock(&nlm_blocked_lock);
        list_for_each_entry(block, &nlm_blocked, b_list) {
                if (nlm_compare_locks(&block->b_call->a_args.lock.fl, fl)) {
-                       dprintk("lockd: nlmsvc_notify_blocked block %p flags 
%d\n",
-                                                       block, block->b_flags);
-                       if (block->b_flags & B_QUEUED) {
-                               if (block->b_flags & B_TIMED_OUT) {
-                                       rc = -ENOLCK;
-                                       break;
-                               }
-                               nlmsvc_update_deferred_block(block, result);
-                       } else if (result == 0)
-                               block->b_granted = 1;
-
-                       nlmsvc_insert_block_locked(block, 0);
-                       svc_wake_up(block->b_daemon);
-                       rc = 0;
+                       kref_get(&block->b_count);
                        break;
                }
        }
        spin_unlock(&nlm_blocked_lock);
-       if (rc == -ENOENT)
-               printk(KERN_WARNING "lockd: grant for unknown block\n");
+
+       if (!block) {
+               pr_warn("lockd: grant for unknown pending block\n");
+               return -ENOENT;
+       }
+
+       /* don't interfere with nlmsvc_lock() */
+       mutex_lock(&block->b_file->f_mutex);
+       block->b_flags &= ~B_PENDING_CALLBACK;
+
+       spin_lock(&nlm_blocked_lock);
+       WARN_ON_ONCE(list_empty(&block->b_list));
+       rc = __nlmsvc_grant_deferred(block, fl, result);
+       spin_unlock(&nlm_blocked_lock);
+       mutex_unlock(&block->b_file->f_mutex);
+
+       nlmsvc_release_block(block);
        return rc;
 }
 
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index f42594a9efe0..a977be8bcc2c 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -189,6 +189,7 @@ struct nlm_block {
 #define B_QUEUED               1       /* lock queued */
 #define B_GOT_CALLBACK         2       /* got lock or conflicting lock */
 #define B_TIMED_OUT            4       /* filesystem too slow to respond */
+#define B_PENDING_CALLBACK     8       /* pending callback for lock request */
 };
 
 /*
-- 
2.31.1

Reply via email to