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 <[email protected]> --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
