cifs: fix time_last_write stamp placement in setattr/truncate paths

cifs_file_set_size() calls cifs_setsize() on success, which calls
i_size_write(), updating i_size to the new value.  The subsequent
check attrs->ia_size != i_size_read() in both cifs_setattr_unix()
and cifs_setattr_nounix() therefore always evaluates false after a
successful cifs_file_set_size(), making the smp_store_release() of
time_last_write dead code.  The truncate path was unprotected against
stale readdir size updates.

Move the stamp to before the cifs_file_set_size() RPC call, guarded
by attrs->ia_size != i_size_read() to exclude no-op same-size
ftruncate(2) calls from stamping time_last_write unnecessarily.

On the error path the stamp remains rather than being restored:
restoring a stale snapshot (prev_tlw) could silently erase a
concurrent _cifsFileInfo_put() close stamp if that close arrived
between the READ_ONCE and the smp_store_release.  readdir is
suppressed until the stamp expires, which extends beyond one acregmax
if the caller retries failed truncations.  stat() is unaffected: the
cifs_revalidate_dentry_attr() path calls cifs_fattr_to_inode() with
from_readdir=false, which bypasses the time_last_write check in
is_size_safe_to_change() entirely and always writes the authoritative
QUERY_INFO result to i_size.

Remove the now-unreachable stamp from the dead block in both functions.

Fixes: e8a8d54c2d50 ("cifs: prevent readdir from changing file size due to stale directory metadata")
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index b2806371bfdeaea766613c4e118f1f10be196bca..808085eb0cdc716fcaba44f064afaec2172d67f8 100644 (file)
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -3190,6 +3190,17 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
        rc = 0;
 
        if (attrs->ia_valid & ATTR_SIZE) {
+               if (attrs->ia_size != i_size_read(inode)) {
+                       /* Stamp before RPC. On failure the stamp remains: restoring a
+                        * stale snapshot could silently erase a concurrent
+                        * _cifsFileInfo_put() close stamp.  readdir is suppressed
+                        * until the stamp expires; stat() bypasses this via the
+                        * from_readdir=false path in is_size_safe_to_change() and
+                        * always returns an authoritative QUERY_INFO result.
+                        * Pairs with smp_load_acquire() in is_size_safe_to_change().
+                        */
+                       smp_store_release(&cifsInode->time_last_write, jiffies);
+               }
                rc = cifs_file_set_size(xid, direntry, full_path,
                                        open_file, attrs->ia_size);
                if (rc != 0)
@@ -3279,8 +3290,6 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 
        if ((attrs->ia_valid & ATTR_SIZE) &&
            attrs->ia_size != i_size_read(inode)) {
-               /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-               smp_store_release(&cifsInode->time_last_write, jiffies);
                truncate_setsize(inode, attrs->ia_size);
                netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
                fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
@@ -3370,6 +3379,17 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
        }
 
        if (attrs->ia_valid & ATTR_SIZE) {
+               if (attrs->ia_size != i_size_read(inode)) {
+                       /* Stamp before RPC. On failure the stamp remains: restoring a
+                        * stale snapshot could silently erase a concurrent
+                        * _cifsFileInfo_put() close stamp.  readdir is suppressed
+                        * until the stamp expires; stat() bypasses this via the
+                        * from_readdir=false path in is_size_safe_to_change() and
+                        * always returns an authoritative QUERY_INFO result.
+                        * Pairs with smp_load_acquire() in is_size_safe_to_change().
+                        */
+                       smp_store_release(&cifsInode->time_last_write, jiffies);
+               }
                rc = cifs_file_set_size(xid, direntry, full_path,
                                        cfile, attrs->ia_size);
                if (rc != 0)
@@ -3482,8 +3502,6 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 
        if ((attrs->ia_valid & ATTR_SIZE) &&
            attrs->ia_size != i_size_read(inode)) {
-               /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-               smp_store_release(&cifsInode->time_last_write, jiffies);
                truncate_setsize(inode, attrs->ia_size);
                netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
                fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);