Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

locks: fix TOCTOU race when granting write lease

Thread A trying to acquire a write lease checks the value of i_readcount
and i_writecount in check_conflicting_open() to verify that its own fd
is the only fd referencing the file.

Thread B trying to open the file for read will call break_lease() in
do_dentry_open() before incrementing i_readcount, which leaves a small
window where thread A can acquire the write lease and then thread B
completes the open of the file for read without breaking the write lease
that was acquired by thread A.

Fix this race by incrementing i_readcount before checking for existing
leases, same as the case with i_writecount.

Use a helper put_file_access() to decrement i_readcount or i_writecount
in do_dentry_open() and __fput().

Fixes: 387e3746d01c ("locks: eliminate false positive conflicts for write lease")
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

authored by

Amir Goldstein and committed by
Al Viro
d6da19c9 568035b0

+15 -13
+1 -6
fs/file_table.c
··· 324 324 } 325 325 fops_put(file->f_op); 326 326 put_pid(file->f_owner.pid); 327 - if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) 328 - i_readcount_dec(inode); 329 - if (mode & FMODE_WRITER) { 330 - put_write_access(inode); 331 - __mnt_drop_write(mnt); 332 - } 327 + put_file_access(file); 333 328 dput(dentry); 334 329 if (unlikely(mode & FMODE_NEED_UNMOUNT)) 335 330 dissolve_on_fput(mnt);
+10
fs/internal.h
··· 101 101 extern struct file *alloc_empty_file(int, const struct cred *); 102 102 extern struct file *alloc_empty_file_noaccount(int, const struct cred *); 103 103 104 + static inline void put_file_access(struct file *file) 105 + { 106 + if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) { 107 + i_readcount_dec(file->f_inode); 108 + } else if (file->f_mode & FMODE_WRITER) { 109 + put_write_access(file->f_inode); 110 + __mnt_drop_write(file->f_path.mnt); 111 + } 112 + } 113 + 104 114 /* 105 115 * super.c 106 116 */
+4 -7
fs/open.c
··· 840 840 return 0; 841 841 } 842 842 843 - if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) { 843 + if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) { 844 + i_readcount_inc(inode); 845 + } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) { 844 846 error = get_write_access(inode); 845 847 if (unlikely(error)) 846 848 goto cleanup_file; ··· 882 880 goto cleanup_all; 883 881 } 884 882 f->f_mode |= FMODE_OPENED; 885 - if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) 886 - i_readcount_inc(inode); 887 883 if ((f->f_mode & FMODE_READ) && 888 884 likely(f->f_op->read || f->f_op->read_iter)) 889 885 f->f_mode |= FMODE_CAN_READ; ··· 935 935 if (WARN_ON_ONCE(error > 0)) 936 936 error = -EINVAL; 937 937 fops_put(f->f_op); 938 - if (f->f_mode & FMODE_WRITER) { 939 - put_write_access(inode); 940 - __mnt_drop_write(f->f_path.mnt); 941 - } 938 + put_file_access(f); 942 939 cleanup_file: 943 940 path_put(&f->f_path); 944 941 f->f_path.mnt = NULL;