From: Alexander Aring <aahringo@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [RFC v6.5-rc2 2/3] fs: lockd: fix race in async lock request handling
Date: Thu, 20 Jul 2023 08:58:05 -0400 [thread overview]
Message-ID: <20230720125806.1385279-2-aahringo@redhat.com> (raw)
In-Reply-To: <20230720125806.1385279-1-aahringo@redhat.com>
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 <aahringo@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
next prev parent reply other threads:[~2023-07-20 12:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-20 12:58 [Cluster-devel] [RFC v6.5-rc2 1/3] fs: lockd: nlm_blocked list race fixes Alexander Aring
2023-07-20 12:58 ` Alexander Aring [this message]
2023-07-21 13:09 ` [Cluster-devel] [RFC v6.5-rc2 2/3] fs: lockd: fix race in async lock request handling Alexander Aring
2023-07-21 16:43 ` Jeff Layton
2023-08-10 21:00 ` Alexander Aring
2023-07-21 15:45 ` Jeff Layton
2023-08-10 20:37 ` Alexander Aring
2023-07-20 12:58 ` [Cluster-devel] [RFC v6.5-rc2 3/3] fs: lockd: introduce safe async lock op Alexander Aring
2023-07-21 17:46 ` Jeff Layton
2023-08-10 20:24 ` Alexander Aring
2023-07-21 15:14 ` [Cluster-devel] [RFC v6.5-rc2 1/3] fs: lockd: nlm_blocked list race fixes Jeff Layton
2023-07-21 16:59 ` Chuck Lever
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230720125806.1385279-2-aahringo@redhat.com \
--to=aahringo@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).