My only worry is that perhaps we should make it more narrow (ie only
when called from readdir ie cifs_prime_dcache() rather than also
never updating it on query_info calls)
On Sun, Feb 25, 2024 at 10:50 PM Bharath SM <bharathsm.hsk@xxxxxxxxx> wrote:
>
> In cases of large directories, the readdir operation may span multiple
> round trips to retrieve contents. This introduces a potential race
> condition in case of concurrent write and readdir operations. If the
> readdir operation initiates before a write has been processed by the
> server, it may update the file size attribute to an older value.
> Address this issue by avoiding file size updates from server when a
> read/write lease.
>
> Scenario:
> 1) process1: open dir xyz
> 2) process1: readdir instance 1 on xyz
> 3) process2: create file.txt for write
> 4) process2: write x bytes to file.txt
> 5) process2: close file.txt
> 6) process2: open file.txt for read
> 7) process1: readdir 2 - overwrites file.txt inode size to 0
> 8) process2: read contents of file.txt - bug, short read with 0 bytes
>
> Signed-off-by: Bharath SM <bharathsm@xxxxxxxxxxxxx>
> ---
> fs/smb/client/file.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index f2db4a1f81ad..e742d0d0e579 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -2952,7 +2952,8 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file)
> if (!cifsInode)
> return true;
>
> - if (is_inode_writable(cifsInode)) {
> + if (is_inode_writable(cifsInode) ||
> + ((cifsInode->oplock & CIFS_CACHE_RW_FLG) != 0)) {
> /* This inode is open for write at least once */
> struct cifs_sb_info *cifs_sb;
>
> --
> 2.34.1
>
--
Thanks,
Steve
