Linux CIFS filesystem development
 help / color / mirror / Atom feed
* [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC
@ 2025-10-06 16:42 Paulo Alcantara
  2025-10-06 16:42 ` [PATCH 2/3] smb: client: fix missing timestamp updates after ftruncate(2) Paulo Alcantara
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Paulo Alcantara @ 2025-10-06 16:42 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..17d3fa9f88fc 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 = DIV_ROUND_UP(size, 512);
 	}
 
 	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 2/3] smb: client: fix missing timestamp updates after ftruncate(2)
  2025-10-06 16:42 [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC Paulo Alcantara
@ 2025-10-06 16:42 ` Paulo Alcantara
  2025-10-06 16:42 ` [PATCH 3/3] smb: client: fix missing timestamp updates after utime(2) Paulo Alcantara
  2025-10-06 17:02 ` [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC Ralph Boehme
  2 siblings, 0 replies; 5+ messages in thread
From: Paulo Alcantara @ 2025-10-06 16:42 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 17d3fa9f88fc..b7245189fc98 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 3/3] smb: client: fix missing timestamp updates after utime(2)
  2025-10-06 16:42 [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC Paulo Alcantara
  2025-10-06 16:42 ` [PATCH 2/3] smb: client: fix missing timestamp updates after ftruncate(2) Paulo Alcantara
@ 2025-10-06 16:42 ` Paulo Alcantara
  2025-10-06 17:02 ` [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC Ralph Boehme
  2 siblings, 0 replies; 5+ messages in thread
From: Paulo Alcantara @ 2025-10-06 16:42 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 | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 0985db9f86e5..89407ca9df9f 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1382,25 +1382,26 @@ 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), };
+	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;
 
 	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)
+			return 0; /* 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,
-- 
2.51.0


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

* Re: [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC
  2025-10-06 16:42 [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC Paulo Alcantara
  2025-10-06 16:42 ` [PATCH 2/3] smb: client: fix missing timestamp updates after ftruncate(2) Paulo Alcantara
  2025-10-06 16:42 ` [PATCH 3/3] smb: client: fix missing timestamp updates after utime(2) Paulo Alcantara
@ 2025-10-06 17:02 ` Ralph Boehme
  2025-10-06 17:05   ` Paulo Alcantara
  2 siblings, 1 reply; 5+ messages in thread
From: Ralph Boehme @ 2025-10-06 17:02 UTC (permalink / raw)
  To: Paulo Alcantara, smfrench; +Cc: Frank Sorenson, David Howells, linux-cifs


[-- Attachment #1.1: Type: text/plain, Size: 16777 bytes --]

Fwiw, I have a patch in-flight that disables sticky write time behavour 
on POSIX handles in Samba.

On 10/6/25 6:42 PM, Paulo Alcantara 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..17d3fa9f88fc 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 = DIV_ROUND_UP(size, 512);
>   	}
>   
>   	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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

* Re: [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC
  2025-10-06 17:02 ` [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC Ralph Boehme
@ 2025-10-06 17:05   ` Paulo Alcantara
  0 siblings, 0 replies; 5+ messages in thread
From: Paulo Alcantara @ 2025-10-06 17:05 UTC (permalink / raw)
  To: Ralph Boehme, smfrench; +Cc: Frank Sorenson, David Howells, linux-cifs

Ralph Boehme <slow@samba.org> writes:

> Fwiw, I have a patch in-flight that disables sticky write time behavour 
> on POSIX handles in Samba.

Awesome!  Thanks for letting me know.

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

end of thread, other threads:[~2025-10-06 17:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-06 16:42 [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC Paulo Alcantara
2025-10-06 16:42 ` [PATCH 2/3] smb: client: fix missing timestamp updates after ftruncate(2) Paulo Alcantara
2025-10-06 16:42 ` [PATCH 3/3] smb: client: fix missing timestamp updates after utime(2) Paulo Alcantara
2025-10-06 17:02 ` [PATCH 1/3] smb: client: fix missing timestamp updates with O_TRUNC Ralph Boehme
2025-10-06 17:05   ` Paulo Alcantara

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