* [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 related [flat|nested] 4+ messages in thread* Re: [PATCH] cifs: flush before set-info if we have writeable handles
2019-07-16 3:23 [PATCH] cifs: flush before set-info if we have writeable handles 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-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 related [flat|nested] 4+ messages in thread* [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 related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-18 22:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-16 3:23 [PATCH] cifs: flush before set-info if we have writeable handles Ronnie Sahlberg
2019-07-17 18:44 ` Pavel Shilovsky
-- strict thread matches above, loose matches on Subject: below --
2019-07-18 22:12 Ronnie Sahlberg
2019-07-16 3:16 Ronnie Sahlberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).