cifs: prevent updating file size from server if we have a read/write lease

[ Upstream commit e4b61f3b1c67f5068590965f64ea6e8d5d5bd961 ]

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 readdir when we
have 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

Cc: stable@vger.kernel.org
Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Bharath SM <bharathsm@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index 9516f57323246ea5495562394a1029bad3257ede..6945551ea166d445d01037da401b6c613f21ec79 100644 (file)
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -144,7 +144,8 @@ extern int cifs_reconnect(struct TCP_Server_Info *server,
 extern int checkSMB(char *buf, unsigned int len, struct TCP_Server_Info *srvr);
 extern bool is_valid_oplock_break(char *, struct TCP_Server_Info *);
 extern bool backup_cred(struct cifs_sb_info *);
-extern bool is_size_safe_to_change(struct cifsInodeInfo *, __u64 eof);
+extern bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 eof,
+                                  bool from_readdir);
 extern void cifs_update_eof(struct cifsInodeInfo *cifsi, loff_t offset,
                            unsigned int bytes_written);
 extern struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *, int);
@@ -201,7 +202,8 @@ extern void cifs_unix_basic_to_fattr(struct cifs_fattr *fattr,
                                     struct cifs_sb_info *cifs_sb);
 extern void cifs_dir_info_to_fattr(struct cifs_fattr *, FILE_DIRECTORY_INFO *,
                                        struct cifs_sb_info *);
-extern int cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr);
+extern int cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr,
+                              bool from_readdir);
 extern struct inode *cifs_iget(struct super_block *sb,
                               struct cifs_fattr *fattr);
 

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index c156460eb558718da6f2cba03e45f670b144f4b7..c711d5eb2987e9925a12361b864a818efe8207da 100644 (file)
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -329,7 +329,7 @@ int cifs_posix_open(const char *full_path, struct inode **pinode,
                }
        } else {
                cifs_revalidate_mapping(*pinode);
-               rc = cifs_fattr_to_inode(*pinode, &fattr);
+               rc = cifs_fattr_to_inode(*pinode, &fattr, false);
        }
 
 posix_open_ret:
@@ -4766,12 +4766,14 @@ static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
    refreshing the inode only on increases in the file size
    but this is tricky to do without racing with writebehind
    page caching in the current Linux kernel design */
-bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file)
+bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file,
+                           bool from_readdir)
 {
        if (!cifsInode)
                return true;
 
-       if (is_inode_writable(cifsInode)) {
+       if (is_inode_writable(cifsInode) ||
+               ((cifsInode->oplock & CIFS_CACHE_RW_FLG) != 0 && from_readdir)) {
                /* This inode is open for write at least once */
                struct cifs_sb_info *cifs_sb;
 

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 471abc99bbf029b6c868eb1a8e3856d11b3a5490..cb9e719e67ae29107544746fe592405a0f1307e3 100644 (file)
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -147,7 +147,8 @@ cifs_nlink_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
 
 /* populate an inode with info from a cifs_fattr struct */
 int
-cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
+cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr,
+                   bool from_readdir)
 {
        struct cifsInodeInfo *cifs_i = CIFS_I(inode);
        struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
@@ -199,7 +200,7 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
         * Can't safely change the file size here if the client is writing to
         * it due to potential races.
         */
-       if (is_size_safe_to_change(cifs_i, fattr->cf_eof)) {
+       if (is_size_safe_to_change(cifs_i, fattr->cf_eof, from_readdir)) {
                i_size_write(inode, fattr->cf_eof);
 
                /*
@@ -368,7 +369,7 @@ static int update_inode_info(struct super_block *sb,
                CIFS_I(*inode)->time = 0; /* force reval */
                return -ESTALE;
        }
-       return cifs_fattr_to_inode(*inode, fattr);
+       return cifs_fattr_to_inode(*inode, fattr, false);
 }
 
 #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
@@ -403,7 +404,7 @@ cifs_get_file_info_unix(struct file *filp)
        } else
                goto cifs_gfiunix_out;
 
-       rc = cifs_fattr_to_inode(inode, &fattr);
+       rc = cifs_fattr_to_inode(inode, &fattr, false);
 
 cifs_gfiunix_out:
        free_xid(xid);
@@ -928,7 +929,7 @@ cifs_get_file_info(struct file *filp)
        fattr.cf_uniqueid = CIFS_I(inode)->uniqueid;
        fattr.cf_flags |= CIFS_FATTR_NEED_REVAL;
        /* if filetype is different, return error */
-       rc = cifs_fattr_to_inode(inode, &fattr);
+       rc = cifs_fattr_to_inode(inode, &fattr, false);
 cgfi_exit:
        cifs_free_open_info(&data);
        free_xid(xid);
@@ -1467,7 +1468,7 @@ retry_iget5_locked:
                }
 
                /* can't fail - see cifs_find_inode() */
-               cifs_fattr_to_inode(inode, fattr);
+               cifs_fattr_to_inode(inode, fattr, false);
                if (sb->s_flags & SB_NOATIME)
                        inode->i_flags |= S_NOATIME | S_NOCMTIME;
                if (inode->i_state & I_NEW) {

diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 520c490e844b55e476a2c43344e8c05a5ef19c7e..56033e4e4bae9ede54cc2cbcf74802efe1b30597 100644 (file)
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -148,7 +148,7 @@ retry:
                                                rc = -ESTALE;
                                        }
                                }
-                               if (!rc && !cifs_fattr_to_inode(inode, fattr)) {
+                               if (!rc && !cifs_fattr_to_inode(inode, fattr, true)) {
                                        dput(dentry);
                                        return;
                                }