Linux CIFS filesystem development
 help / color / mirror / Atom feed
* [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC
@ 2025-10-07 19:23 Paulo Alcantara
  2025-10-07 19:23 ` [PATCH v3 2/4] smb: client: fix missing timestamp updates after ftruncate(2) Paulo Alcantara
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Paulo Alcantara @ 2025-10-07 19:23 UTC (permalink / raw)
  To: smfrench
  Cc: Frank Sorenson, Paulo Alcantara (Red Hat), David Howells,
	linux-cifs

Don't call ->set_file_info() on open handle to prevent the server from
stopping [cm]time updates automatically as per MS-FSA 2.1.4.17.

Fix this by checking for ATTR_OPEN bit earlier in cifs_setattr() to
prevent ->set_file_info() from being called when opening a file with
O_TRUNC.  Do the truncation in ->open() instead.

This also saves two roundtrips when opening a file with O_TRUNC and
there are currently no open handles to be reused.

Before patch:

$ mount.cifs //srv/share /mnt -o ...
$ cd /mnt
$ exec 3>foo; stat -c 'old: %z %y' foo; sleep 2; echo test >&3; exec 3>&-; sleep 2; stat -c 'new: %z %y' foo
old: 2025-10-03 13:26:23.151030500 -0300 2025-10-03 13:26:23.151030500 -0300
new: 2025-10-03 13:26:23.151030500 -0300 2025-10-03 13:26:23.151030500 -0300

After patch:

$ mount.cifs //srv/share /mnt -o ...
$ cd /mnt
$ exec 3>foo; stat -c 'old: %z %y' foo; sleep 2; echo test >&3; exec 3>&-; sleep 2; stat -c 'new: %z %y' foo
$ exec 3>foo; stat -c 'old: %z %y' foo; sleep 2; echo test >&3; exec 3>&-; sleep 2; stat -c 'new: %z %y' foo
old: 2025-10-03 13:28:13.911933800 -0300 2025-10-03 13:28:13.911933800 -0300
new: 2025-10-03 13:28:26.647492700 -0300 2025-10-03 13:28:26.647492700 -0300

Reported-by: Frank Sorenson <sorenson@redhat.com>
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Cc: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
---
 fs/smb/client/cifsglob.h |   5 ++
 fs/smb/client/file.c     |  94 +++++++++++++++++-------
 fs/smb/client/inode.c    | 152 ++++++++++++++++++++++-----------------
 3 files changed, 159 insertions(+), 92 deletions(-)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 3ac254e123dc..8f6f567d7474 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1566,6 +1566,11 @@ struct cifsFileInfo *cifsFileInfo_get(struct cifsFileInfo *cifs_file);
 void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_hdlr,
 		       bool offload);
 void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
+int cifs_file_flush(const unsigned int xid, struct inode *inode,
+		    struct cifsFileInfo *cfile);
+int cifs_file_set_size(const unsigned int xid, struct dentry *dentry,
+		       const char *full_path, struct cifsFileInfo *open_file,
+		       loff_t size);
 
 #define CIFS_CACHE_READ_FLG	1
 #define CIFS_CACHE_HANDLE_FLG	2
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index a5ed742afa00..ecbb63e66521 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -952,6 +952,66 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
 	}
 }
 
+int cifs_file_flush(const unsigned int xid, struct inode *inode,
+		    struct cifsFileInfo *cfile)
+{
+	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	struct cifs_tcon *tcon;
+	int rc;
+
+	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC)
+		return 0;
+
+	if (cfile && (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE)) {
+		tcon = tlink_tcon(cfile->tlink);
+		return tcon->ses->server->ops->flush(xid, tcon,
+						     &cfile->fid);
+	}
+	rc = cifs_get_writable_file(CIFS_I(inode), FIND_WR_ANY, &cfile);
+	if (!rc) {
+		tcon = tlink_tcon(cfile->tlink);
+		rc = tcon->ses->server->ops->flush(xid, tcon, &cfile->fid);
+		cifsFileInfo_put(cfile);
+	} else if (rc == -EBADF) {
+		rc = 0;
+	}
+	return rc;
+}
+
+static int cifs_do_truncate(const unsigned int xid, struct dentry *dentry)
+{
+	struct cifsInodeInfo *cinode = CIFS_I(d_inode(dentry));
+	struct inode *inode = d_inode(dentry);
+	struct cifsFileInfo *cfile = NULL;
+	struct TCP_Server_Info *server;
+	struct cifs_tcon *tcon;
+	int rc;
+
+	rc = filemap_write_and_wait(inode->i_mapping);
+	if (is_interrupt_error(rc))
+		return -ERESTARTSYS;
+	mapping_set_error(inode->i_mapping, rc);
+
+	cfile = find_writable_file(cinode, FIND_WR_FSUID_ONLY);
+	rc = cifs_file_flush(xid, inode, cfile);
+	if (!rc) {
+		if (cfile) {
+			tcon = tlink_tcon(cfile->tlink);
+			server = tcon->ses->server;
+			rc = server->ops->set_file_size(xid, tcon,
+							cfile, 0, false);
+		}
+		if (!rc) {
+			netfs_resize_file(&cinode->netfs, 0, true);
+			cifs_setsize(inode, 0);
+			inode->i_blocks = 0;
+		}
+	}
+	if (cfile)
+		cifsFileInfo_put(cfile);
+	return rc;
+}
+
 int cifs_open(struct inode *inode, struct file *file)
 
 {
@@ -1004,6 +1064,12 @@ int cifs_open(struct inode *inode, struct file *file)
 			file->f_op = &cifs_file_direct_ops;
 	}
 
+	if (file->f_flags & O_TRUNC) {
+		rc = cifs_do_truncate(xid, file_dentry(file));
+		if (rc)
+			goto out;
+	}
+
 	/* Get the cached handle as SMB2 close is deferred */
 	if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) {
 		rc = cifs_get_writable_path(tcon, full_path,
@@ -2685,13 +2751,10 @@ cifs_get_readable_path(struct cifs_tcon *tcon, const char *name,
 int cifs_strict_fsync(struct file *file, loff_t start, loff_t end,
 		      int datasync)
 {
-	unsigned int xid;
-	int rc = 0;
-	struct cifs_tcon *tcon;
-	struct TCP_Server_Info *server;
 	struct cifsFileInfo *smbfile = file->private_data;
 	struct inode *inode = file_inode(file);
-	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	unsigned int xid;
+	int rc = 0;
 
 	rc = file_write_and_wait_range(file, start, end);
 	if (rc) {
@@ -2712,26 +2775,7 @@ int cifs_strict_fsync(struct file *file, loff_t start, loff_t end,
 		}
 	}
 
-	tcon = tlink_tcon(smbfile->tlink);
-	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC)) {
-		server = tcon->ses->server;
-		if (server->ops->flush == NULL) {
-			rc = -ENOSYS;
-			goto strict_fsync_exit;
-		}
-
-		if ((OPEN_FMODE(smbfile->f_flags) & FMODE_WRITE) == 0) {
-			smbfile = find_writable_file(CIFS_I(inode), FIND_WR_ANY);
-			if (smbfile) {
-				rc = server->ops->flush(xid, tcon, &smbfile->fid);
-				cifsFileInfo_put(smbfile);
-			} else
-				cifs_dbg(FYI, "ignore fsync for file not open for write\n");
-		} else
-			rc = server->ops->flush(xid, tcon, &smbfile->fid);
-	}
-
-strict_fsync_exit:
+	rc = cifs_file_flush(xid, inode, smbfile);
 	free_xid(xid);
 	return rc;
 }
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 8bb544be401e..ce88bef4e44c 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -3007,28 +3007,24 @@ int cifs_fiemap(struct inode *inode, struct fiemap_extent_info *fei, u64 start,
 
 void cifs_setsize(struct inode *inode, loff_t offset)
 {
-	struct cifsInodeInfo *cifs_i = CIFS_I(inode);
-
 	spin_lock(&inode->i_lock);
 	i_size_write(inode, offset);
 	spin_unlock(&inode->i_lock);
-
-	/* Cached inode must be refreshed on truncate */
-	cifs_i->time = 0;
+	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
 	truncate_pagecache(inode, offset);
 }
 
-static int
-cifs_set_file_size(struct inode *inode, struct iattr *attrs,
-		   unsigned int xid, const char *full_path, struct dentry *dentry)
+int cifs_file_set_size(const unsigned int xid, struct dentry *dentry,
+		       const char *full_path, struct cifsFileInfo *open_file,
+		       loff_t size)
 {
-	int rc;
-	struct cifsFileInfo *open_file;
-	struct cifsInodeInfo *cifsInode = CIFS_I(inode);
+	struct inode *inode = d_inode(dentry);
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	struct cifsInodeInfo *cifsInode = CIFS_I(inode);
 	struct tcon_link *tlink = NULL;
 	struct cifs_tcon *tcon = NULL;
 	struct TCP_Server_Info *server;
+	int rc = -EINVAL;
 
 	/*
 	 * To avoid spurious oplock breaks from server, in the case of
@@ -3039,19 +3035,25 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs,
 	 * writebehind data than the SMB timeout for the SetPathInfo
 	 * request would allow
 	 */
-	open_file = find_writable_file(cifsInode, FIND_WR_FSUID_ONLY);
-	if (open_file) {
+	if (open_file && (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE)) {
 		tcon = tlink_tcon(open_file->tlink);
 		server = tcon->ses->server;
-		if (server->ops->set_file_size)
-			rc = server->ops->set_file_size(xid, tcon, open_file,
-							attrs->ia_size, false);
-		else
-			rc = -ENOSYS;
-		cifsFileInfo_put(open_file);
-		cifs_dbg(FYI, "SetFSize for attrs rc = %d\n", rc);
-	} else
-		rc = -EINVAL;
+		rc = server->ops->set_file_size(xid, tcon,
+						open_file,
+						size, false);
+		cifs_dbg(FYI, "%s: set_file_size: rc = %d\n", __func__, rc);
+	} else {
+		open_file = find_writable_file(cifsInode, FIND_WR_FSUID_ONLY);
+		if (open_file) {
+			tcon = tlink_tcon(open_file->tlink);
+			server = tcon->ses->server;
+			rc = server->ops->set_file_size(xid, tcon,
+							open_file,
+							size, false);
+			cifs_dbg(FYI, "%s: set_file_size: rc = %d\n", __func__, rc);
+			cifsFileInfo_put(open_file);
+		}
+	}
 
 	if (!rc)
 		goto set_size_out;
@@ -3069,20 +3071,15 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs,
 	 * valid, writeable file handle for it was found or because there was
 	 * an error setting it by handle.
 	 */
-	if (server->ops->set_path_size)
-		rc = server->ops->set_path_size(xid, tcon, full_path,
-						attrs->ia_size, cifs_sb, false, dentry);
-	else
-		rc = -ENOSYS;
-	cifs_dbg(FYI, "SetEOF by path (setattrs) rc = %d\n", rc);
-
-	if (tlink)
-		cifs_put_tlink(tlink);
+	rc = server->ops->set_path_size(xid, tcon, full_path, size,
+					cifs_sb, false, dentry);
+	cifs_dbg(FYI, "%s: SetEOF by path (setattrs) rc = %d\n", __func__, rc);
+	cifs_put_tlink(tlink);
 
 set_size_out:
 	if (rc == 0) {
-		netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
-		cifs_setsize(inode, attrs->ia_size);
+		netfs_resize_file(&cifsInode->netfs, size, true);
+		cifs_setsize(inode, size);
 		/*
 		 * i_blocks is not related to (i_size / i_blksize), but instead
 		 * 512 byte (2**9) size is required for calculating num blocks.
@@ -3090,15 +3087,7 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs,
 		 * this is best estimate we have for blocks allocated for a file
 		 * Number of blocks must be rounded up so size 1 is not 0 blocks
 		 */
-		inode->i_blocks = (512 - 1 + attrs->ia_size) >> 9;
-
-		/*
-		 * The man page of truncate says if the size changed,
-		 * then the st_ctime and st_mtime fields for the file
-		 * are updated.
-		 */
-		attrs->ia_ctime = attrs->ia_mtime = current_time(inode);
-		attrs->ia_valid |= ATTR_CTIME | ATTR_MTIME;
+		inode->i_blocks = (512 - 1 + size) >> 9;
 	}
 
 	return rc;
@@ -3118,7 +3107,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 	struct tcon_link *tlink;
 	struct cifs_tcon *pTcon;
 	struct cifs_unix_set_info_args *args = NULL;
-	struct cifsFileInfo *open_file;
+	struct cifsFileInfo *open_file = NULL;
 
 	cifs_dbg(FYI, "setattr_unix on file %pd attrs->ia_valid=0x%x\n",
 		 direntry, attrs->ia_valid);
@@ -3132,6 +3121,9 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 	if (rc < 0)
 		goto out;
 
+	if (attrs->ia_valid & ATTR_FILE)
+		open_file = attrs->ia_file->private_data;
+
 	full_path = build_path_from_dentry(direntry, page);
 	if (IS_ERR(full_path)) {
 		rc = PTR_ERR(full_path);
@@ -3159,9 +3151,17 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 	rc = 0;
 
 	if (attrs->ia_valid & ATTR_SIZE) {
-		rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry);
+		rc = cifs_file_set_size(xid, direntry, full_path,
+					open_file, attrs->ia_size);
 		if (rc != 0)
 			goto out;
+		/*
+		 * The man page of truncate says if the size changed,
+		 * then the st_ctime and st_mtime fields for the file
+		 * are updated.
+		 */
+		attrs->ia_ctime = attrs->ia_mtime = current_time(inode);
+		attrs->ia_valid |= ATTR_CTIME | ATTR_MTIME;
 	}
 
 	/* skip mode change if it's just for clearing setuid/setgid */
@@ -3206,14 +3206,24 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 		args->ctime = NO_CHANGE_64;
 
 	args->device = 0;
-	open_file = find_writable_file(cifsInode, FIND_WR_FSUID_ONLY);
-	if (open_file) {
-		u16 nfid = open_file->fid.netfid;
-		u32 npid = open_file->pid;
+	rc = -EINVAL;
+	if (open_file && (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE)) {
 		pTcon = tlink_tcon(open_file->tlink);
-		rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args, nfid, npid);
-		cifsFileInfo_put(open_file);
+		rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args,
+					    open_file->fid.netfid,
+					    open_file->pid);
 	} else {
+		open_file = find_writable_file(cifsInode, FIND_WR_FSUID_ONLY);
+		if (open_file) {
+			pTcon = tlink_tcon(open_file->tlink);
+			rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args,
+						    open_file->fid.netfid,
+						    open_file->pid);
+			cifsFileInfo_put(open_file);
+		}
+	}
+
+	if (rc) {
 		tlink = cifs_sb_tlink(cifs_sb);
 		if (IS_ERR(tlink)) {
 			rc = PTR_ERR(tlink);
@@ -3221,8 +3231,8 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 		}
 		pTcon = tlink_tcon(tlink);
 		rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args,
-				    cifs_sb->local_nls,
-				    cifs_remap(cifs_sb));
+					    cifs_sb->local_nls,
+					    cifs_remap(cifs_sb));
 		cifs_put_tlink(tlink);
 	}
 
@@ -3264,8 +3274,7 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 	struct inode *inode = d_inode(direntry);
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 	struct cifsInodeInfo *cifsInode = CIFS_I(inode);
-	struct cifsFileInfo *wfile;
-	struct cifs_tcon *tcon;
+	struct cifsFileInfo *cfile = NULL;
 	const char *full_path;
 	void *page = alloc_dentry_path();
 	int rc = -EACCES;
@@ -3285,6 +3294,9 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 	if (rc < 0)
 		goto cifs_setattr_exit;
 
+	if (attrs->ia_valid & ATTR_FILE)
+		cfile = attrs->ia_file->private_data;
+
 	full_path = build_path_from_dentry(direntry, page);
 	if (IS_ERR(full_path)) {
 		rc = PTR_ERR(full_path);
@@ -3311,25 +3323,24 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 
 	rc = 0;
 
-	if ((attrs->ia_valid & ATTR_MTIME) &&
-	    !(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC)) {
-		rc = cifs_get_writable_file(cifsInode, FIND_WR_ANY, &wfile);
-		if (!rc) {
-			tcon = tlink_tcon(wfile->tlink);
-			rc = tcon->ses->server->ops->flush(xid, tcon, &wfile->fid);
-			cifsFileInfo_put(wfile);
-			if (rc)
-				goto cifs_setattr_exit;
-		} else if (rc != -EBADF)
+	if (attrs->ia_valid & ATTR_MTIME) {
+		rc = cifs_file_flush(xid, inode, cfile);
+		if (rc)
 			goto cifs_setattr_exit;
-		else
-			rc = 0;
 	}
 
 	if (attrs->ia_valid & ATTR_SIZE) {
-		rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry);
+		rc = cifs_file_set_size(xid, direntry, full_path,
+					cfile, attrs->ia_size);
 		if (rc != 0)
 			goto cifs_setattr_exit;
+		/*
+		 * The man page of truncate says if the size changed,
+		 * then the st_ctime and st_mtime fields for the file
+		 * are updated.
+		 */
+		attrs->ia_ctime = attrs->ia_mtime = current_time(inode);
+		attrs->ia_valid |= ATTR_CTIME | ATTR_MTIME;
 	}
 
 	if (attrs->ia_valid & ATTR_UID)
@@ -3459,6 +3470,13 @@ cifs_setattr(struct mnt_idmap *idmap, struct dentry *direntry,
 
 	if (unlikely(cifs_forced_shutdown(cifs_sb)))
 		return -EIO;
+	/*
+	 * Avoid setting [cm]time with O_TRUNC to prevent the server from
+	 * disabling automatic timestamp updates as specified in
+	 * MS-FSA 2.1.4.17.
+	 */
+	if (attrs->ia_valid & ATTR_OPEN)
+		return 0;
 
 	do {
 #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/4] smb: client: fix missing timestamp updates after ftruncate(2)
  2025-10-07 19:23 [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC Paulo Alcantara
@ 2025-10-07 19:23 ` Paulo Alcantara
  2025-10-07 19:23 ` [PATCH v3 3/4] smb: client: fix missing timestamp updates after utime(2) Paulo Alcantara
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Paulo Alcantara @ 2025-10-07 19:23 UTC (permalink / raw)
  To: smfrench
  Cc: Paulo Alcantara (Red Hat), Frank Sorenson, David Howells,
	linux-cifs

Mask off ATTR_MTIME|ATTR_CTIME bits on ATTR_SIZE (e.g. ftruncate(2))
to prevent the client from sending set info calls and then disabling
automatic timestamp updates on server side as per MS-FSA 2.1.4.17.

---8<---
import os
import time

filename = '/mnt/foo'

def print_stat(prefix):
    st = os.stat(filename)
    print(prefix, ': ', time.ctime(st.st_atime), time.ctime(st.st_ctime))

fd = os.open(filename, os.O_CREAT|os.O_TRUNC|os.O_WRONLY, 0o644)
print_stat('old')
os.ftruncate(fd, 10)
time.sleep(2)
os.write(fd, b'foo')
os.close(fd)
time.sleep(2)
print_stat('new')
---8<---

Before patch:

$ mount.cifs //srv/share /mnt -o ...
$ python3 run.py
old :  Fri Oct  3 13:47:03 2025 Fri Oct  3 13:47:03 2025
new :  Fri Oct  3 13:47:00 2025 Fri Oct  3 13:47:03 2025

After patch:

$ mount.cifs //srv/share /mnt -o ...
$ python3 run.py
old :  Fri Oct  3 13:48:39 2025 Fri Oct  3 13:48:39 2025
new :  Fri Oct  3 13:48:41 2025 Fri Oct  3 13:48:41 2025

Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Cc: Frank Sorenson <sorenson@redhat.com>
Cc: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
---
 fs/smb/client/inode.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index ce88bef4e44c..fbfd5b556815 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -3156,12 +3156,11 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 		if (rc != 0)
 			goto out;
 		/*
-		 * The man page of truncate says if the size changed,
-		 * then the st_ctime and st_mtime fields for the file
-		 * are updated.
+		 * Avoid setting timestamps on the server for ftruncate(2) to
+		 * prevent it from disabling automatic timestamp updates as per
+		 * MS-FSA 2.1.4.17.
 		 */
-		attrs->ia_ctime = attrs->ia_mtime = current_time(inode);
-		attrs->ia_valid |= ATTR_CTIME | ATTR_MTIME;
+		attrs->ia_valid &= ~(ATTR_CTIME | ATTR_MTIME);
 	}
 
 	/* skip mode change if it's just for clearing setuid/setgid */
@@ -3335,12 +3334,11 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 		if (rc != 0)
 			goto cifs_setattr_exit;
 		/*
-		 * The man page of truncate says if the size changed,
-		 * then the st_ctime and st_mtime fields for the file
-		 * are updated.
+		 * Avoid setting timestamps on the server for ftruncate(2) to
+		 * prevent it from disabling automatic timestamp updates as per
+		 * MS-FSA 2.1.4.17.
 		 */
-		attrs->ia_ctime = attrs->ia_mtime = current_time(inode);
-		attrs->ia_valid |= ATTR_CTIME | ATTR_MTIME;
+		attrs->ia_valid &= ~(ATTR_CTIME | ATTR_MTIME);
 	}
 
 	if (attrs->ia_valid & ATTR_UID)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 3/4] smb: client: fix missing timestamp updates after utime(2)
  2025-10-07 19:23 [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC Paulo Alcantara
  2025-10-07 19:23 ` [PATCH v3 2/4] smb: client: fix missing timestamp updates after ftruncate(2) Paulo Alcantara
@ 2025-10-07 19:23 ` Paulo Alcantara
  2025-10-07 19:23 ` [PATCH v3 4/4] smb: client: fix race with fallocate(2) and AIO+DIO Paulo Alcantara
  2025-10-07 22:29 ` [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC Steve French
  3 siblings, 0 replies; 5+ messages in thread
From: Paulo Alcantara @ 2025-10-07 19:23 UTC (permalink / raw)
  To: smfrench
  Cc: Paulo Alcantara (Red Hat), Frank Sorenson, David Howells,
	linux-cifs

Don't reuse open handle when changing timestamps to prevent the server
from disabling automatic timestamp updates as per MS-FSA 2.1.4.17.

---8<---
import os
import time

filename = '/mnt/foo'

def print_stat(prefix):
    st = os.stat(filename)
    print(prefix, ': ', time.ctime(st.st_atime), time.ctime(st.st_ctime))

fd = os.open(filename, os.O_CREAT|os.O_TRUNC|os.O_WRONLY, 0o644)
print_stat('old')
os.utime(fd, None)
time.sleep(2)
os.write(fd, b'foo')
os.close(fd)
time.sleep(2)
print_stat('new')
---8<---

Before patch:

$ mount.cifs //srv/share /mnt -o ...
$ python3 run.py
old :  Fri Oct  3 14:01:21 2025 Fri Oct  3 14:01:21 2025
new :  Fri Oct  3 14:01:21 2025 Fri Oct  3 14:01:21 2025

After patch:

$ mount.cifs //srv/share /mnt -o ...
$ python3 run.py
old :  Fri Oct  3 17:03:34 2025 Fri Oct  3 17:03:34 2025
new :  Fri Oct  3 17:03:36 2025 Fri Oct  3 17:03:36 2025

Fixes: b6f2a0f89d7e ("cifs: for compound requests, use open handle if possible")
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Cc: Frank Sorenson <sorenson@redhat.com>
Cc: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
---
 fs/smb/client/smb2inode.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 0985db9f86e5..e441fa2e7689 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1382,31 +1382,33 @@ int
 smb2_set_file_info(struct inode *inode, const char *full_path,
 		   FILE_BASIC_INFO *buf, const unsigned int xid)
 {
-	struct cifs_open_parms oparms;
-	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
-	struct tcon_link *tlink;
-	struct cifs_tcon *tcon;
-	struct cifsFileInfo *cfile;
 	struct kvec in_iov = { .iov_base = buf, .iov_len = sizeof(*buf), };
-	int rc;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	struct cifsFileInfo *cfile = NULL;
+	struct cifs_open_parms oparms;
+	struct tcon_link *tlink;
+	struct cifs_tcon *tcon;
+	int rc = 0;
+
+	tlink = cifs_sb_tlink(cifs_sb);
+	if (IS_ERR(tlink))
+		return PTR_ERR(tlink);
+	tcon = tlink_tcon(tlink);
 
 	if ((buf->CreationTime == 0) && (buf->LastAccessTime == 0) &&
-	    (buf->LastWriteTime == 0) && (buf->ChangeTime == 0) &&
-	    (buf->Attributes == 0))
-		return 0; /* would be a no op, no sense sending this */
+	    (buf->LastWriteTime == 0) && (buf->ChangeTime == 0)) {
+		if (buf->Attributes == 0)
+			goto out; /* would be a no op, no sense sending this */
+		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
+	}
 
-	tlink = cifs_sb_tlink(cifs_sb);
-	if (IS_ERR(tlink))
-		return PTR_ERR(tlink);
-	tcon = tlink_tcon(tlink);
-
-	cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
 	oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, FILE_WRITE_ATTRIBUTES,
 			     FILE_OPEN, 0, ACL_NO_MODE);
 	rc = smb2_compound_op(xid, tcon, cifs_sb,
 			      full_path, &oparms, &in_iov,
 			      &(int){SMB2_OP_SET_INFO}, 1,
 			      cfile, NULL, NULL, NULL);
+out:
 	cifs_put_tlink(tlink);
 	return rc;
 }
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 4/4] smb: client: fix race with fallocate(2) and AIO+DIO
  2025-10-07 19:23 [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC Paulo Alcantara
  2025-10-07 19:23 ` [PATCH v3 2/4] smb: client: fix missing timestamp updates after ftruncate(2) Paulo Alcantara
  2025-10-07 19:23 ` [PATCH v3 3/4] smb: client: fix missing timestamp updates after utime(2) Paulo Alcantara
@ 2025-10-07 19:23 ` Paulo Alcantara
  2025-10-07 22:29 ` [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC Steve French
  3 siblings, 0 replies; 5+ messages in thread
From: Paulo Alcantara @ 2025-10-07 19:23 UTC (permalink / raw)
  To: smfrench
  Cc: Paulo Alcantara (Red Hat), David Howells, Frank Sorenson,
	linux-cifs

AIO+DIO may extend the file size, hence we need to make sure ->i_size
is stable across the entire fallocate(2) operation, otherwise it would
become a truncate and then inode size reduced back down when it
finishes.

Fix this by calling netfs_wait_for_outstanding_io() right after
acquiring ->i_rwsem exclusively in cifs_fallocate() and then guarantee
a stable ->i_size across fallocate(2).

Also call netfs_wait_for_outstanding_io() after truncating pagecache
to avoid any potential races with writeback.

Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Frank Sorenson <sorenson@redhat.com>
Cc: linux-cifs@vger.kernel.org
---
 fs/smb/client/cifsfs.c  | 22 +++++++++++++++++++---
 fs/smb/client/inode.c   |  1 +
 fs/smb/client/smb2ops.c | 18 ++++++------------
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 1775c2b7528f..05b1fa76e8cc 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -392,11 +392,27 @@ static long cifs_fallocate(struct file *file, int mode, loff_t off, loff_t len)
 	struct cifs_sb_info *cifs_sb = CIFS_FILE_SB(file);
 	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
 	struct TCP_Server_Info *server = tcon->ses->server;
+	struct inode *inode = file_inode(file);
+	int rc;
 
-	if (server->ops->fallocate)
-		return server->ops->fallocate(file, tcon, mode, off, len);
+	if (!server->ops->fallocate)
+		return -EOPNOTSUPP;
 
-	return -EOPNOTSUPP;
+	rc = inode_lock_killable(inode);
+	if (rc)
+		return rc;
+
+	netfs_wait_for_outstanding_io(inode);
+
+	rc = file_modified(file);
+	if (rc)
+		goto out_unlock;
+
+	rc = server->ops->fallocate(file, tcon, mode, off, len);
+
+out_unlock:
+	inode_unlock(inode);
+	return rc;
 }
 
 static int cifs_permission(struct mnt_idmap *idmap,
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index fbfd5b556815..239dd84a336f 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -3012,6 +3012,7 @@ void cifs_setsize(struct inode *inode, loff_t offset)
 	spin_unlock(&inode->i_lock);
 	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
 	truncate_pagecache(inode, offset);
+	netfs_wait_for_outstanding_io(inode);
 }
 
 int cifs_file_set_size(const unsigned int xid, struct dentry *dentry,
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 80114292e2c9..6226dc787860 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -3367,7 +3367,6 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
 	trace_smb3_zero_enter(xid, cfile->fid.persistent_fid, tcon->tid,
 			      ses->Suid, offset, len);
 
-	inode_lock(inode);
 	filemap_invalidate_lock(inode->i_mapping);
 
 	i_size = i_size_read(inode);
@@ -3385,6 +3384,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
 	 * first, otherwise the data may be inconsistent with the server.
 	 */
 	truncate_pagecache_range(inode, offset, offset + len - 1);
+	netfs_wait_for_outstanding_io(inode);
 
 	/* if file not oplocked can't be sure whether asking to extend size */
 	rc = -EOPNOTSUPP;
@@ -3413,7 +3413,6 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
 
  zero_range_exit:
 	filemap_invalidate_unlock(inode->i_mapping);
-	inode_unlock(inode);
 	free_xid(xid);
 	if (rc)
 		trace_smb3_zero_err(xid, cfile->fid.persistent_fid, tcon->tid,
@@ -3437,7 +3436,6 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
 
 	xid = get_xid();
 
-	inode_lock(inode);
 	/* Need to make file sparse, if not already, before freeing range. */
 	/* Consider adding equivalent for compressed since it could also work */
 	if (!smb2_set_sparse(xid, tcon, cfile, inode, set_sparse)) {
@@ -3451,6 +3449,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
 	 * caches first, otherwise the data may be inconsistent with the server.
 	 */
 	truncate_pagecache_range(inode, offset, offset + len - 1);
+	netfs_wait_for_outstanding_io(inode);
 
 	cifs_dbg(FYI, "Offset %lld len %lld\n", offset, len);
 
@@ -3485,7 +3484,6 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
 unlock:
 	filemap_invalidate_unlock(inode->i_mapping);
 out:
-	inode_unlock(inode);
 	free_xid(xid);
 	return rc;
 }
@@ -3749,8 +3747,6 @@ static long smb3_collapse_range(struct file *file, struct cifs_tcon *tcon,
 
 	xid = get_xid();
 
-	inode_lock(inode);
-
 	old_eof = i_size_read(inode);
 	if ((off >= old_eof) ||
 	    off + len >= old_eof) {
@@ -3765,6 +3761,7 @@ static long smb3_collapse_range(struct file *file, struct cifs_tcon *tcon,
 
 	truncate_pagecache_range(inode, off, old_eof);
 	ictx->zero_point = old_eof;
+	netfs_wait_for_outstanding_io(inode);
 
 	rc = smb2_copychunk_range(xid, cfile, cfile, off + len,
 				  old_eof - off - len, off);
@@ -3785,8 +3782,7 @@ static long smb3_collapse_range(struct file *file, struct cifs_tcon *tcon,
 	fscache_resize_cookie(cifs_inode_cookie(inode), new_eof);
 out_2:
 	filemap_invalidate_unlock(inode->i_mapping);
- out:
-	inode_unlock(inode);
+out:
 	free_xid(xid);
 	return rc;
 }
@@ -3803,8 +3799,6 @@ static long smb3_insert_range(struct file *file, struct cifs_tcon *tcon,
 
 	xid = get_xid();
 
-	inode_lock(inode);
-
 	old_eof = i_size_read(inode);
 	if (off >= old_eof) {
 		rc = -EINVAL;
@@ -3819,6 +3813,7 @@ static long smb3_insert_range(struct file *file, struct cifs_tcon *tcon,
 	if (rc < 0)
 		goto out_2;
 	truncate_pagecache_range(inode, off, old_eof);
+	netfs_wait_for_outstanding_io(inode);
 
 	rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
 			  cfile->fid.volatile_fid, cfile->pid, new_eof);
@@ -3841,8 +3836,7 @@ static long smb3_insert_range(struct file *file, struct cifs_tcon *tcon,
 	rc = 0;
 out_2:
 	filemap_invalidate_unlock(inode->i_mapping);
- out:
-	inode_unlock(inode);
+out:
 	free_xid(xid);
 	return rc;
 }
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC
  2025-10-07 19:23 [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC Paulo Alcantara
                   ` (2 preceding siblings ...)
  2025-10-07 19:23 ` [PATCH v3 4/4] smb: client: fix race with fallocate(2) and AIO+DIO Paulo Alcantara
@ 2025-10-07 22:29 ` Steve French
  3 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2025-10-07 22:29 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: Frank Sorenson, David Howells, linux-cifs

Tentatively merged into cifs-2.6.git for-next pending additional
review/testing (also added the RB from David Howells)

On Tue, Oct 7, 2025 at 2:23 PM Paulo Alcantara <pc@manguebit.org> wrote:
>
> Don't call ->set_file_info() on open handle to prevent the server from
> stopping [cm]time updates automatically as per MS-FSA 2.1.4.17.
>
> Fix this by checking for ATTR_OPEN bit earlier in cifs_setattr() to
> prevent ->set_file_info() from being called when opening a file with
> O_TRUNC.  Do the truncation in ->open() instead.
>
> This also saves two roundtrips when opening a file with O_TRUNC and
> there are currently no open handles to be reused.
>
> Before patch:
>
> $ mount.cifs //srv/share /mnt -o ...
> $ cd /mnt
> $ exec 3>foo; stat -c 'old: %z %y' foo; sleep 2; echo test >&3; exec 3>&-; sleep 2; stat -c 'new: %z %y' foo
> old: 2025-10-03 13:26:23.151030500 -0300 2025-10-03 13:26:23.151030500 -0300
> new: 2025-10-03 13:26:23.151030500 -0300 2025-10-03 13:26:23.151030500 -0300
>
> After patch:
>
> $ mount.cifs //srv/share /mnt -o ...
> $ cd /mnt
> $ exec 3>foo; stat -c 'old: %z %y' foo; sleep 2; echo test >&3; exec 3>&-; sleep 2; stat -c 'new: %z %y' foo
> $ exec 3>foo; stat -c 'old: %z %y' foo; sleep 2; echo test >&3; exec 3>&-; sleep 2; stat -c 'new: %z %y' foo
> old: 2025-10-03 13:28:13.911933800 -0300 2025-10-03 13:28:13.911933800 -0300
> new: 2025-10-03 13:28:26.647492700 -0300 2025-10-03 13:28:26.647492700 -0300
>
> Reported-by: Frank Sorenson <sorenson@redhat.com>
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> Cc: David Howells <dhowells@redhat.com>
> Cc: linux-cifs@vger.kernel.org
> ---
>  fs/smb/client/cifsglob.h |   5 ++
>  fs/smb/client/file.c     |  94 +++++++++++++++++-------
>  fs/smb/client/inode.c    | 152 ++++++++++++++++++++++-----------------
>  3 files changed, 159 insertions(+), 92 deletions(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index 3ac254e123dc..8f6f567d7474 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -1566,6 +1566,11 @@ struct cifsFileInfo *cifsFileInfo_get(struct cifsFileInfo *cifs_file);
>  void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_hdlr,
>                        bool offload);
>  void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
> +int cifs_file_flush(const unsigned int xid, struct inode *inode,
> +                   struct cifsFileInfo *cfile);
> +int cifs_file_set_size(const unsigned int xid, struct dentry *dentry,
> +                      const char *full_path, struct cifsFileInfo *open_file,
> +                      loff_t size);
>
>  #define CIFS_CACHE_READ_FLG    1
>  #define CIFS_CACHE_HANDLE_FLG  2
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index a5ed742afa00..ecbb63e66521 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -952,6 +952,66 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
>         }
>  }
>
> +int cifs_file_flush(const unsigned int xid, struct inode *inode,
> +                   struct cifsFileInfo *cfile)
> +{
> +       struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> +       struct cifs_tcon *tcon;
> +       int rc;
> +
> +       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC)
> +               return 0;
> +
> +       if (cfile && (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE)) {
> +               tcon = tlink_tcon(cfile->tlink);
> +               return tcon->ses->server->ops->flush(xid, tcon,
> +                                                    &cfile->fid);
> +       }
> +       rc = cifs_get_writable_file(CIFS_I(inode), FIND_WR_ANY, &cfile);
> +       if (!rc) {
> +               tcon = tlink_tcon(cfile->tlink);
> +               rc = tcon->ses->server->ops->flush(xid, tcon, &cfile->fid);
> +               cifsFileInfo_put(cfile);
> +       } else if (rc == -EBADF) {
> +               rc = 0;
> +       }
> +       return rc;
> +}
> +
> +static int cifs_do_truncate(const unsigned int xid, struct dentry *dentry)
> +{
> +       struct cifsInodeInfo *cinode = CIFS_I(d_inode(dentry));
> +       struct inode *inode = d_inode(dentry);
> +       struct cifsFileInfo *cfile = NULL;
> +       struct TCP_Server_Info *server;
> +       struct cifs_tcon *tcon;
> +       int rc;
> +
> +       rc = filemap_write_and_wait(inode->i_mapping);
> +       if (is_interrupt_error(rc))
> +               return -ERESTARTSYS;
> +       mapping_set_error(inode->i_mapping, rc);
> +
> +       cfile = find_writable_file(cinode, FIND_WR_FSUID_ONLY);
> +       rc = cifs_file_flush(xid, inode, cfile);
> +       if (!rc) {
> +               if (cfile) {
> +                       tcon = tlink_tcon(cfile->tlink);
> +                       server = tcon->ses->server;
> +                       rc = server->ops->set_file_size(xid, tcon,
> +                                                       cfile, 0, false);
> +               }
> +               if (!rc) {
> +                       netfs_resize_file(&cinode->netfs, 0, true);
> +                       cifs_setsize(inode, 0);
> +                       inode->i_blocks = 0;
> +               }
> +       }
> +       if (cfile)
> +               cifsFileInfo_put(cfile);
> +       return rc;
> +}
> +
>  int cifs_open(struct inode *inode, struct file *file)
>
>  {
> @@ -1004,6 +1064,12 @@ int cifs_open(struct inode *inode, struct file *file)
>                         file->f_op = &cifs_file_direct_ops;
>         }
>
> +       if (file->f_flags & O_TRUNC) {
> +               rc = cifs_do_truncate(xid, file_dentry(file));
> +               if (rc)
> +                       goto out;
> +       }
> +
>         /* Get the cached handle as SMB2 close is deferred */
>         if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) {
>                 rc = cifs_get_writable_path(tcon, full_path,
> @@ -2685,13 +2751,10 @@ cifs_get_readable_path(struct cifs_tcon *tcon, const char *name,
>  int cifs_strict_fsync(struct file *file, loff_t start, loff_t end,
>                       int datasync)
>  {
> -       unsigned int xid;
> -       int rc = 0;
> -       struct cifs_tcon *tcon;
> -       struct TCP_Server_Info *server;
>         struct cifsFileInfo *smbfile = file->private_data;
>         struct inode *inode = file_inode(file);
> -       struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> +       unsigned int xid;
> +       int rc = 0;
>
>         rc = file_write_and_wait_range(file, start, end);
>         if (rc) {
> @@ -2712,26 +2775,7 @@ int cifs_strict_fsync(struct file *file, loff_t start, loff_t end,
>                 }
>         }
>
> -       tcon = tlink_tcon(smbfile->tlink);
> -       if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC)) {
> -               server = tcon->ses->server;
> -               if (server->ops->flush == NULL) {
> -                       rc = -ENOSYS;
> -                       goto strict_fsync_exit;
> -               }
> -
> -               if ((OPEN_FMODE(smbfile->f_flags) & FMODE_WRITE) == 0) {
> -                       smbfile = find_writable_file(CIFS_I(inode), FIND_WR_ANY);
> -                       if (smbfile) {
> -                               rc = server->ops->flush(xid, tcon, &smbfile->fid);
> -                               cifsFileInfo_put(smbfile);
> -                       } else
> -                               cifs_dbg(FYI, "ignore fsync for file not open for write\n");
> -               } else
> -                       rc = server->ops->flush(xid, tcon, &smbfile->fid);
> -       }
> -
> -strict_fsync_exit:
> +       rc = cifs_file_flush(xid, inode, smbfile);
>         free_xid(xid);
>         return rc;
>  }
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index 8bb544be401e..ce88bef4e44c 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -3007,28 +3007,24 @@ int cifs_fiemap(struct inode *inode, struct fiemap_extent_info *fei, u64 start,
>
>  void cifs_setsize(struct inode *inode, loff_t offset)
>  {
> -       struct cifsInodeInfo *cifs_i = CIFS_I(inode);
> -
>         spin_lock(&inode->i_lock);
>         i_size_write(inode, offset);
>         spin_unlock(&inode->i_lock);
> -
> -       /* Cached inode must be refreshed on truncate */
> -       cifs_i->time = 0;
> +       inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
>         truncate_pagecache(inode, offset);
>  }
>
> -static int
> -cifs_set_file_size(struct inode *inode, struct iattr *attrs,
> -                  unsigned int xid, const char *full_path, struct dentry *dentry)
> +int cifs_file_set_size(const unsigned int xid, struct dentry *dentry,
> +                      const char *full_path, struct cifsFileInfo *open_file,
> +                      loff_t size)
>  {
> -       int rc;
> -       struct cifsFileInfo *open_file;
> -       struct cifsInodeInfo *cifsInode = CIFS_I(inode);
> +       struct inode *inode = d_inode(dentry);
>         struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> +       struct cifsInodeInfo *cifsInode = CIFS_I(inode);
>         struct tcon_link *tlink = NULL;
>         struct cifs_tcon *tcon = NULL;
>         struct TCP_Server_Info *server;
> +       int rc = -EINVAL;
>
>         /*
>          * To avoid spurious oplock breaks from server, in the case of
> @@ -3039,19 +3035,25 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs,
>          * writebehind data than the SMB timeout for the SetPathInfo
>          * request would allow
>          */
> -       open_file = find_writable_file(cifsInode, FIND_WR_FSUID_ONLY);
> -       if (open_file) {
> +       if (open_file && (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE)) {
>                 tcon = tlink_tcon(open_file->tlink);
>                 server = tcon->ses->server;
> -               if (server->ops->set_file_size)
> -                       rc = server->ops->set_file_size(xid, tcon, open_file,
> -                                                       attrs->ia_size, false);
> -               else
> -                       rc = -ENOSYS;
> -               cifsFileInfo_put(open_file);
> -               cifs_dbg(FYI, "SetFSize for attrs rc = %d\n", rc);
> -       } else
> -               rc = -EINVAL;
> +               rc = server->ops->set_file_size(xid, tcon,
> +                                               open_file,
> +                                               size, false);
> +               cifs_dbg(FYI, "%s: set_file_size: rc = %d\n", __func__, rc);
> +       } else {
> +               open_file = find_writable_file(cifsInode, FIND_WR_FSUID_ONLY);
> +               if (open_file) {
> +                       tcon = tlink_tcon(open_file->tlink);
> +                       server = tcon->ses->server;
> +                       rc = server->ops->set_file_size(xid, tcon,
> +                                                       open_file,
> +                                                       size, false);
> +                       cifs_dbg(FYI, "%s: set_file_size: rc = %d\n", __func__, rc);
> +                       cifsFileInfo_put(open_file);
> +               }
> +       }
>
>         if (!rc)
>                 goto set_size_out;
> @@ -3069,20 +3071,15 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs,
>          * valid, writeable file handle for it was found or because there was
>          * an error setting it by handle.
>          */
> -       if (server->ops->set_path_size)
> -               rc = server->ops->set_path_size(xid, tcon, full_path,
> -                                               attrs->ia_size, cifs_sb, false, dentry);
> -       else
> -               rc = -ENOSYS;
> -       cifs_dbg(FYI, "SetEOF by path (setattrs) rc = %d\n", rc);
> -
> -       if (tlink)
> -               cifs_put_tlink(tlink);
> +       rc = server->ops->set_path_size(xid, tcon, full_path, size,
> +                                       cifs_sb, false, dentry);
> +       cifs_dbg(FYI, "%s: SetEOF by path (setattrs) rc = %d\n", __func__, rc);
> +       cifs_put_tlink(tlink);
>
>  set_size_out:
>         if (rc == 0) {
> -               netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
> -               cifs_setsize(inode, attrs->ia_size);
> +               netfs_resize_file(&cifsInode->netfs, size, true);
> +               cifs_setsize(inode, size);
>                 /*
>                  * i_blocks is not related to (i_size / i_blksize), but instead
>                  * 512 byte (2**9) size is required for calculating num blocks.
> @@ -3090,15 +3087,7 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs,
>                  * this is best estimate we have for blocks allocated for a file
>                  * Number of blocks must be rounded up so size 1 is not 0 blocks
>                  */
> -               inode->i_blocks = (512 - 1 + attrs->ia_size) >> 9;
> -
> -               /*
> -                * The man page of truncate says if the size changed,
> -                * then the st_ctime and st_mtime fields for the file
> -                * are updated.
> -                */
> -               attrs->ia_ctime = attrs->ia_mtime = current_time(inode);
> -               attrs->ia_valid |= ATTR_CTIME | ATTR_MTIME;
> +               inode->i_blocks = (512 - 1 + size) >> 9;
>         }
>
>         return rc;
> @@ -3118,7 +3107,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
>         struct tcon_link *tlink;
>         struct cifs_tcon *pTcon;
>         struct cifs_unix_set_info_args *args = NULL;
> -       struct cifsFileInfo *open_file;
> +       struct cifsFileInfo *open_file = NULL;
>
>         cifs_dbg(FYI, "setattr_unix on file %pd attrs->ia_valid=0x%x\n",
>                  direntry, attrs->ia_valid);
> @@ -3132,6 +3121,9 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
>         if (rc < 0)
>                 goto out;
>
> +       if (attrs->ia_valid & ATTR_FILE)
> +               open_file = attrs->ia_file->private_data;
> +
>         full_path = build_path_from_dentry(direntry, page);
>         if (IS_ERR(full_path)) {
>                 rc = PTR_ERR(full_path);
> @@ -3159,9 +3151,17 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
>         rc = 0;
>
>         if (attrs->ia_valid & ATTR_SIZE) {
> -               rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry);
> +               rc = cifs_file_set_size(xid, direntry, full_path,
> +                                       open_file, attrs->ia_size);
>                 if (rc != 0)
>                         goto out;
> +               /*
> +                * The man page of truncate says if the size changed,
> +                * then the st_ctime and st_mtime fields for the file
> +                * are updated.
> +                */
> +               attrs->ia_ctime = attrs->ia_mtime = current_time(inode);
> +               attrs->ia_valid |= ATTR_CTIME | ATTR_MTIME;
>         }
>
>         /* skip mode change if it's just for clearing setuid/setgid */
> @@ -3206,14 +3206,24 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
>                 args->ctime = NO_CHANGE_64;
>
>         args->device = 0;
> -       open_file = find_writable_file(cifsInode, FIND_WR_FSUID_ONLY);
> -       if (open_file) {
> -               u16 nfid = open_file->fid.netfid;
> -               u32 npid = open_file->pid;
> +       rc = -EINVAL;
> +       if (open_file && (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE)) {
>                 pTcon = tlink_tcon(open_file->tlink);
> -               rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args, nfid, npid);
> -               cifsFileInfo_put(open_file);
> +               rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args,
> +                                           open_file->fid.netfid,
> +                                           open_file->pid);
>         } else {
> +               open_file = find_writable_file(cifsInode, FIND_WR_FSUID_ONLY);
> +               if (open_file) {
> +                       pTcon = tlink_tcon(open_file->tlink);
> +                       rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args,
> +                                                   open_file->fid.netfid,
> +                                                   open_file->pid);
> +                       cifsFileInfo_put(open_file);
> +               }
> +       }
> +
> +       if (rc) {
>                 tlink = cifs_sb_tlink(cifs_sb);
>                 if (IS_ERR(tlink)) {
>                         rc = PTR_ERR(tlink);
> @@ -3221,8 +3231,8 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
>                 }
>                 pTcon = tlink_tcon(tlink);
>                 rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args,
> -                                   cifs_sb->local_nls,
> -                                   cifs_remap(cifs_sb));
> +                                           cifs_sb->local_nls,
> +                                           cifs_remap(cifs_sb));
>                 cifs_put_tlink(tlink);
>         }
>
> @@ -3264,8 +3274,7 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
>         struct inode *inode = d_inode(direntry);
>         struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
>         struct cifsInodeInfo *cifsInode = CIFS_I(inode);
> -       struct cifsFileInfo *wfile;
> -       struct cifs_tcon *tcon;
> +       struct cifsFileInfo *cfile = NULL;
>         const char *full_path;
>         void *page = alloc_dentry_path();
>         int rc = -EACCES;
> @@ -3285,6 +3294,9 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
>         if (rc < 0)
>                 goto cifs_setattr_exit;
>
> +       if (attrs->ia_valid & ATTR_FILE)
> +               cfile = attrs->ia_file->private_data;
> +
>         full_path = build_path_from_dentry(direntry, page);
>         if (IS_ERR(full_path)) {
>                 rc = PTR_ERR(full_path);
> @@ -3311,25 +3323,24 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
>
>         rc = 0;
>
> -       if ((attrs->ia_valid & ATTR_MTIME) &&
> -           !(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC)) {
> -               rc = cifs_get_writable_file(cifsInode, FIND_WR_ANY, &wfile);
> -               if (!rc) {
> -                       tcon = tlink_tcon(wfile->tlink);
> -                       rc = tcon->ses->server->ops->flush(xid, tcon, &wfile->fid);
> -                       cifsFileInfo_put(wfile);
> -                       if (rc)
> -                               goto cifs_setattr_exit;
> -               } else if (rc != -EBADF)
> +       if (attrs->ia_valid & ATTR_MTIME) {
> +               rc = cifs_file_flush(xid, inode, cfile);
> +               if (rc)
>                         goto cifs_setattr_exit;
> -               else
> -                       rc = 0;
>         }
>
>         if (attrs->ia_valid & ATTR_SIZE) {
> -               rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry);
> +               rc = cifs_file_set_size(xid, direntry, full_path,
> +                                       cfile, attrs->ia_size);
>                 if (rc != 0)
>                         goto cifs_setattr_exit;
> +               /*
> +                * The man page of truncate says if the size changed,
> +                * then the st_ctime and st_mtime fields for the file
> +                * are updated.
> +                */
> +               attrs->ia_ctime = attrs->ia_mtime = current_time(inode);
> +               attrs->ia_valid |= ATTR_CTIME | ATTR_MTIME;
>         }
>
>         if (attrs->ia_valid & ATTR_UID)
> @@ -3459,6 +3470,13 @@ cifs_setattr(struct mnt_idmap *idmap, struct dentry *direntry,
>
>         if (unlikely(cifs_forced_shutdown(cifs_sb)))
>                 return -EIO;
> +       /*
> +        * Avoid setting [cm]time with O_TRUNC to prevent the server from
> +        * disabling automatic timestamp updates as specified in
> +        * MS-FSA 2.1.4.17.
> +        */
> +       if (attrs->ia_valid & ATTR_OPEN)
> +               return 0;
>
>         do {
>  #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
> --
> 2.51.0
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-10-07 22:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-07 19:23 [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC Paulo Alcantara
2025-10-07 19:23 ` [PATCH v3 2/4] smb: client: fix missing timestamp updates after ftruncate(2) Paulo Alcantara
2025-10-07 19:23 ` [PATCH v3 3/4] smb: client: fix missing timestamp updates after utime(2) Paulo Alcantara
2025-10-07 19:23 ` [PATCH v3 4/4] smb: client: fix race with fallocate(2) and AIO+DIO Paulo Alcantara
2025-10-07 22:29 ` [PATCH v3 1/4] smb: client: fix missing timestamp updates with O_TRUNC Steve French

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox