[PATCH 3/4] locks: when upgrading, don't remove old flock lock until replacing with new one

[Jeff Layton <jeff.layton@primarydata.com>]

There is a potential problem when upgrading a flock lock. Suppose we
have a LOCK_SH lock on a file, and then want to upgrade it to a LOCK_EX
lock. We go through the first loop in flock_lock_file, and remove the
first lock.

We then go through the second loop and try to insert a new LOCK_EX lock.
If however, there is another LOCK_SH lock on the file, we're out of
luck. We've removed our LOCK_SH lock already and can't insert a LOCK_EX
lock.

Fix this by ensuring that we don't remove any lock that we're replacing
until we're sure that we can add its replacement.

Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/locks.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 00c105f499a2..59eadd416b8c 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -864,13 +864,16 @@ static int posix_locks_deadlock(struct file_lock *caller_fl,
 static int flock_lock_file(struct file *filp, struct file_lock *request)
 {
 	struct file_lock *new_fl = NULL;
+	struct file_lock *old_fl = NULL;
 	struct file_lock *fl;
 	struct file_lock_context *ctx;
 	struct inode *inode = file_inode(filp);
 	int error = 0;
-	bool found = false;
 	LIST_HEAD(dispose);
 
+	/* flock_locks_conflict relies on this */
+	WARN_ON_ONCE(request->fl_file != filp);
+
 	ctx = locks_get_lock_context(inode);
 	if (!ctx)
 		return -ENOMEM;
@@ -885,22 +888,29 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
 	if (request->fl_flags & FL_ACCESS)
 		goto find_conflict;
 
+	/*
+	 * Do we already hold a lock on this filp? It may be upgradeable, or it
+	 * may be just what we need.
+	 */
 	list_for_each_entry(fl, &ctx->flc_flock, fl_list) {
 		if (filp != fl->fl_file)
 			continue;
 		if (request->fl_type == fl->fl_type)
 			goto out;
-		found = true;
-		locks_delete_lock_ctx(fl, &dispose);
+		old_fl = fl;
 		break;
 	}
 
 	if (request->fl_type == F_UNLCK) {
-		if ((request->fl_flags & FL_EXISTS) && !found)
-			error = -ENOENT;
+		if (old_fl) {
+			locks_delete_lock_ctx(old_fl, &dispose);
+			if (request->fl_flags & FL_EXISTS)
+				error = -ENOENT;
+		}
 		goto out;
 	}
 
+	/* SETLK(W) */
 find_conflict:
 	list_for_each_entry(fl, &ctx->flc_flock, fl_list) {
 		if (!flock_locks_conflict(request, fl))
@@ -915,6 +925,8 @@ find_conflict:
 	if (request->fl_flags & FL_ACCESS)
 		goto out;
 	locks_copy_lock(new_fl, request);
+	if (old_fl)
+		locks_delete_lock_ctx(old_fl, &dispose);
 	locks_insert_lock_ctx(new_fl, &ctx->flc_flock);
 	new_fl = NULL;
 	error = 0;
-- 
2.1.0