Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] cifs: flush before set-info if we have writeable handles
@ 2019-07-16  3:16 Ronnie Sahlberg
  0 siblings, 0 replies; 4+ messages in thread
From: Ronnie Sahlberg @ 2019-07-16  3:16 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French, Ronnie Sahlberg

Servers can defer destaging any data and updating the mtime until close().
This means that if we do a setinfo to modify the mtime while other handles
are open for write the server may overwrite our setinfo timestamps when
if flushes the file on close() of the writeable handle.

To avoid this we add an explicit flush() before any attempts to use setinfo
and update the mtime IF we have writeable handles open.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2inode.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 278405d26c47..a32a993077ea 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -516,7 +516,11 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
 {
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 	struct tcon_link *tlink;
+	struct cifs_tcon *tcon;
+	struct TCP_Server_Info *server;
 	int rc;
+	struct cifsInodeInfo *cifsi;
+	struct cifsFileInfo *wrcfile;
 
 	if ((buf->CreationTime == 0) && (buf->LastAccessTime == 0) &&
 	    (buf->LastWriteTime == 0) && (buf->ChangeTime == 0) &&
@@ -527,7 +531,18 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
 	if (IS_ERR(tlink))
 		return PTR_ERR(tlink);
 
-	rc = smb2_compound_op(xid, tlink_tcon(tlink), cifs_sb, full_path,
+	tcon = tlink_tcon(tlink);
+	server = tcon->ses->server;
+
+	cifsi = CIFS_I(inode);
+	wrcfile = find_writable_file(cifsi, false);
+	if (wrcfile) {
+		filemap_write_and_wait(inode->i_mapping);
+		server->ops->flush(xid, tcon, &wrcfile->fid);
+		cifsFileInfo_put(wrcfile);
+	}
+
+	rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
 			      FILE_WRITE_ATTRIBUTES, FILE_OPEN, 0, buf,
 			      SMB2_OP_SET_INFO);
 	cifs_put_tlink(tlink);
-- 
2.13.6


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

* [PATCH] cifs: flush before set-info if we have writeable handles
@ 2019-07-18 22:12 Ronnie Sahlberg
  0 siblings, 0 replies; 4+ messages in thread
From: Ronnie Sahlberg @ 2019-07-18 22:12 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French, Ronnie Sahlberg, Stable

Servers can defer destaging any data and updating the mtime until close().
This means that if we do a setinfo to modify the mtime while other handles
are open for write the server may overwrite our setinfo timestamps when
if flushes the file on close() of the writeable handle.

To solve this we add an explicit flush when the mtime is about to
be updated.

This fixes "cp -p" to preserve mtime when copying a file onto an SMB2 share.

CC: Stable <stable@vger.kernel.org>
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/inode.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 1bffe029fb66..56ca4b8ccaba 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -2406,6 +2406,8 @@ 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;
 	char *full_path = NULL;
 	int rc = -EACCES;
 	__u32 dosattr = 0;
@@ -2452,6 +2454,20 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 	mapping_set_error(inode->i_mapping, rc);
 	rc = 0;
 
+	if (attrs->ia_valid & ATTR_MTIME) {
+		rc = cifs_get_writable_file(cifsInode, false, &wfile);
+		if (!rc) {
+			tcon = tlink_tcon(wfile->tlink);
+			rc = tcon->ses->server->ops->flush(xid, tcon, &wfile->fid);
+			cifsFileInfo_put(wfile);
+			if (rc)
+				return rc;
+		} else if (rc != -EBADF)
+			return rc;
+		else
+			rc = 0;
+	}
+
 	if (attrs->ia_valid & ATTR_SIZE) {
 		rc = cifs_set_file_size(inode, attrs, xid, full_path);
 		if (rc != 0)
-- 
2.13.6


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

* Re: [PATCH] cifs: flush before set-info if we have writeable handles
  2019-07-16  3:23 Ronnie Sahlberg
@ 2019-07-17 18:44 ` Pavel Shilovsky
  0 siblings, 0 replies; 4+ messages in thread
From: Pavel Shilovsky @ 2019-07-17 18:44 UTC (permalink / raw)
  To: Ronnie Sahlberg; +Cc: linux-cifs, Steve French

пн, 15 июл. 2019 г. в 20:24, Ronnie Sahlberg <lsahlber@redhat.com>:
>
> Servers can defer destaging any data and updating the mtime until close().
> This means that if we do a setinfo to modify the mtime while other handles
> are open for write the server may overwrite our setinfo timestamps when
> if flushes the file on close() of the writeable handle.
>
> To avoid this we add an explicit flush() before any attempts to use setinfo
> and update the mtime IF we have writeable handles open.
>
> This makes "cp -p" preserve the mtime when copying files to some smb servers.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/smb2inode.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
> index 278405d26c47..a3603ec3a086 100644
> --- a/fs/cifs/smb2inode.c
> +++ b/fs/cifs/smb2inode.c
> @@ -516,7 +516,11 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
>  {
>         struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
>         struct tcon_link *tlink;
> +       struct cifs_tcon *tcon;
> +       struct TCP_Server_Info *server;
>         int rc;
> +       struct cifsInodeInfo *cifsi;
> +       struct cifsFileInfo *wrcfile;
>
>         if ((buf->CreationTime == 0) && (buf->LastAccessTime == 0) &&
>             (buf->LastWriteTime == 0) && (buf->ChangeTime == 0) &&
> @@ -527,7 +531,19 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
>         if (IS_ERR(tlink))
>                 return PTR_ERR(tlink);
>
> -       rc = smb2_compound_op(xid, tlink_tcon(tlink), cifs_sb, full_path,
> +       tcon = tlink_tcon(tlink);
> +       server = tcon->ses->server;
> +
> +       if (buf->LastWriteTime) {
> +               cifsi = CIFS_I(inode);
> +               wrcfile = find_writable_file(cifsi, false);

use get_writable_file instead - this will allow us to get a real RC.
if -EBADF is returned then no writable handle and should proceed
normally, otherwise let's fail setattr.

> +               if (wrcfile) {
> +                       filemap_write_and_wait(inode->i_mapping);

If the above function returns error, the same problem that you are
trying to fix appears. If an error happen, we should fail setattr to
avoid corruption of timestamps.

> +                       server->ops->flush(xid, tcon, &wrcfile->fid);

the same logic applies to flush call - need to fail setattr if an error occured.

> +                       cifsFileInfo_put(wrcfile);
> +               }
> +       }
> +       rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
>                               FILE_WRITE_ATTRIBUTES, FILE_OPEN, 0, buf,
>                               SMB2_OP_SET_INFO);
>         cifs_put_tlink(tlink);
> --
> 2.13.6
>


--
Best regards,
Pavel Shilovsky

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

* [PATCH] cifs: flush before set-info if we have writeable handles
@ 2019-07-16  3:23 Ronnie Sahlberg
  2019-07-17 18:44 ` Pavel Shilovsky
  0 siblings, 1 reply; 4+ messages in thread
From: Ronnie Sahlberg @ 2019-07-16  3:23 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French, Ronnie Sahlberg

Servers can defer destaging any data and updating the mtime until close().
This means that if we do a setinfo to modify the mtime while other handles
are open for write the server may overwrite our setinfo timestamps when
if flushes the file on close() of the writeable handle.

To avoid this we add an explicit flush() before any attempts to use setinfo
and update the mtime IF we have writeable handles open.

This makes "cp -p" preserve the mtime when copying files to some smb servers.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2inode.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 278405d26c47..a3603ec3a086 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -516,7 +516,11 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
 {
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 	struct tcon_link *tlink;
+	struct cifs_tcon *tcon;
+	struct TCP_Server_Info *server;
 	int rc;
+	struct cifsInodeInfo *cifsi;
+	struct cifsFileInfo *wrcfile;
 
 	if ((buf->CreationTime == 0) && (buf->LastAccessTime == 0) &&
 	    (buf->LastWriteTime == 0) && (buf->ChangeTime == 0) &&
@@ -527,7 +531,19 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
 	if (IS_ERR(tlink))
 		return PTR_ERR(tlink);
 
-	rc = smb2_compound_op(xid, tlink_tcon(tlink), cifs_sb, full_path,
+	tcon = tlink_tcon(tlink);
+	server = tcon->ses->server;
+
+	if (buf->LastWriteTime) {
+		cifsi = CIFS_I(inode);
+		wrcfile = find_writable_file(cifsi, false);
+		if (wrcfile) {
+			filemap_write_and_wait(inode->i_mapping);
+			server->ops->flush(xid, tcon, &wrcfile->fid);
+			cifsFileInfo_put(wrcfile);
+		}
+	}
+	rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
 			      FILE_WRITE_ATTRIBUTES, FILE_OPEN, 0, buf,
 			      SMB2_OP_SET_INFO);
 	cifs_put_tlink(tlink);
-- 
2.13.6


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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-16  3:16 [PATCH] cifs: flush before set-info if we have writeable handles Ronnie Sahlberg
2019-07-16  3:23 Ronnie Sahlberg
2019-07-17 18:44 ` Pavel Shilovsky
2019-07-18 22:12 Ronnie Sahlberg

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org linux-cifs@archiver.kernel.org
	public-inbox-index linux-cifs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-cifs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox