Re: [PATCH] locks: fix TOCTOU race when granting write lease

[Al Viro <viro@zeniv.linux.org.uk>]

On Sun, Aug 14, 2022 at 06:23:22PM +0300, Amir Goldstein wrote:
> 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")
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks sane; I'd probably collapsed cleanup_file and cleanup_all while we are
at it, but then I can do that in a followup as well.

> +static inline void put_file_access(struct file *file)
> +{
> +	if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> +		i_readcount_dec(file->f_inode);
> +	} else if (file->f_mode & FMODE_WRITER) {
> +		put_write_access(file->f_inode);
> +		__mnt_drop_write(file->f_path.mnt);
> +	}
> +}

What's the point of having it in linux/fs.h instead of internal.h?