| Message ID | 20240723-fs-lock-recover-compatfix-v1-1-148096719529@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | filelock: Fix fcntl/close race recovery compat path | expand |
On Tue, 23 Jul 2024 17:03:56 +0200, Jann Horn wrote: > When I wrote commit 3cad1bc01041 ("filelock: Remove locks reliably when > fcntl/close race is detected"), I missed that there are two copies of the > code I was patching: The normal version, and the version for 64-bit offsets > on 32-bit kernels. > Thanks to Greg KH for stumbling over this while doing the stable > backport... > > [...] Thanks for fixing the compat path as well! --- Applied to the vfs.fixes branch of the vfs/vfs.git tree. Patches in the vfs.fixes branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.fixes [1/1] filelock: Fix fcntl/close race recovery compat path https://git.kernel.org/vfs/vfs/c/4bc443404754
On Tue, 2024-07-23 at 17:03 +0200, Jann Horn wrote: > When I wrote commit 3cad1bc01041 ("filelock: Remove locks reliably when > fcntl/close race is detected"), I missed that there are two copies of the > code I was patching: The normal version, and the version for 64-bit offsets > on 32-bit kernels. > Thanks to Greg KH for stumbling over this while doing the stable > backport... > > Apply exactly the same fix to the compat path for 32-bit kernels. > > Fixes: c293621bbf67 ("[PATCH] stale POSIX lock handling") > Cc: stable@kernel.org > Link: https://bugs.chromium.org/p/project-zero/issues/detail?id=2563 > Signed-off-by: Jann Horn <jannh@google.com> > --- > fs/locks.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/fs/locks.c b/fs/locks.c > index bdd94c32256f..9afb16e0683f 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -2570,8 +2570,9 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, > error = do_lock_file_wait(filp, cmd, file_lock); > > /* > - * Attempt to detect a close/fcntl race and recover by releasing the > - * lock that was just acquired. There is no need to do that when we're > + * Detect close/fcntl races and recover by zapping all POSIX locks > + * associated with this file and our files_struct, just like on > + * filp_flush(). There is no need to do that when we're > * unlocking though, or for OFD locks. > */ > if (!error && file_lock->c.flc_type != F_UNLCK && > @@ -2586,9 +2587,7 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, > f = files_lookup_fd_locked(files, fd); > spin_unlock(&files->file_lock); > if (f != filp) { > - file_lock->c.flc_type = F_UNLCK; > - error = do_lock_file_wait(filp, cmd, file_lock); > - WARN_ON_ONCE(error); > + locks_remove_posix(filp, files); > error = -EBADF; > } > } > > --- > base-commit: 66ebbdfdeb093e097399b1883390079cd4c3022b > change-id: 20240723-fs-lock-recover-compatfix-cf2cbab343d1 Doh! Good catch. Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/fs/locks.c b/fs/locks.c index bdd94c32256f..9afb16e0683f 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2570,8 +2570,9 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, error = do_lock_file_wait(filp, cmd, file_lock); /* - * Attempt to detect a close/fcntl race and recover by releasing the - * lock that was just acquired. There is no need to do that when we're + * Detect close/fcntl races and recover by zapping all POSIX locks + * associated with this file and our files_struct, just like on + * filp_flush(). There is no need to do that when we're * unlocking though, or for OFD locks. */ if (!error && file_lock->c.flc_type != F_UNLCK && @@ -2586,9 +2587,7 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, f = files_lookup_fd_locked(files, fd); spin_unlock(&files->file_lock); if (f != filp) { - file_lock->c.flc_type = F_UNLCK; - error = do_lock_file_wait(filp, cmd, file_lock); - WARN_ON_ONCE(error); + locks_remove_posix(filp, files); error = -EBADF; } }
When I wrote commit 3cad1bc01041 ("filelock: Remove locks reliably when fcntl/close race is detected"), I missed that there are two copies of the code I was patching: The normal version, and the version for 64-bit offsets on 32-bit kernels. Thanks to Greg KH for stumbling over this while doing the stable backport... Apply exactly the same fix to the compat path for 32-bit kernels. Fixes: c293621bbf67 ("[PATCH] stale POSIX lock handling") Cc: stable@kernel.org Link: https://bugs.chromium.org/p/project-zero/issues/detail?id=2563 Signed-off-by: Jann Horn <jannh@google.com> --- fs/locks.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) --- base-commit: 66ebbdfdeb093e097399b1883390079cd4c3022b change-id: 20240723-fs-lock-recover-compatfix-cf2cbab343d1