[focal:linux][PATCH 2/2] smb3: query attributes on file close
Thadeu Lima de Souza Cascardo
cascardo at canonical.com
Thu Sep 24 15:42:01 UTC 2020
On Wed, Sep 23, 2020 at 02:31:26PM -0300, Marcelo Henrique Cerri wrote:
> From: Steve French <stfrench at microsoft.com>
>
> BugLink: https://bugs.launchpad.net/bugs/1896642
>
> Since timestamps on files on most servers can be updated at
> close, and since timestamps on our dentries default to one
> second we can have stale timestamps in some common cases
> (e.g. open, write, close, stat, wait one second, stat - will
> show different mtime for the first and second stat).
Why isn't this test case being exercised? That is, write/close/stat, and
verify mtime?
Cascardo.
>
> The SMB2/SMB3 protocol allows querying timestamps at close
> so add the code to request timestamp and attr information
> (which is cheap for the server to provide) to be returned
> when a file is closed (it is not needed for the many
> paths that call SMB2_close that are from compounded
> query infos and close nor is it needed for some of
> the cases where a directory close immediately follows a
> directory open.
>
> Signed-off-by: Steve French <stfrench at microsoft.com>
> Acked-by: Ronnie Sahlberg <lsahlber at redhat.com>
> Reviewed-by: Aurelien Aptel <aaptel at suse.com>
> Reviewed-by: Pavel Shilovsky <pshilov at microsoft.com>
> (cherry picked from commit 43f8a6a74ee2442b9410ed297f5d4c77e7cb5ace)
> Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri at canonical.com>
> ---
> fs/cifs/cifsglob.h | 3 +++
> fs/cifs/file.c | 4 +++-
> fs/cifs/smb2inode.c | 2 +-
> fs/cifs/smb2ops.c | 49 +++++++++++++++++++++++++++++++++++++++++----
> fs/cifs/smb2pdu.c | 38 +++++++++++++++++++++++++++--------
> fs/cifs/smb2pdu.h | 11 ++++++++++
> fs/cifs/smb2proto.h | 5 ++++-
> 7 files changed, 97 insertions(+), 15 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index f9cbdfc1591b..84f86e200fe3 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -366,6 +366,9 @@ struct smb_version_operations {
> /* close a file */
> void (*close)(const unsigned int, struct cifs_tcon *,
> struct cifs_fid *);
> + /* close a file, returning file attributes and timestamps */
> + void (*close_getattr)(const unsigned int xid, struct cifs_tcon *tcon,
> + struct cifsFileInfo *pfile_info);
> /* send a flush request to the server */
> int (*flush)(const unsigned int, struct cifs_tcon *, struct cifs_fid *);
> /* async read from the server */
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 4959dbe740f7..9268940cbefd 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -496,7 +496,9 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
> unsigned int xid;
>
> xid = get_xid();
> - if (server->ops->close)
> + if (server->ops->close_getattr)
> + server->ops->close_getattr(xid, tcon, cifs_file);
> + else if (server->ops->close)
> server->ops->close(xid, tcon, &cifs_file->fid);
> _free_xid(xid);
> }
> diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
> index f2a6f7f28340..72d2200a2c5a 100644
> --- a/fs/cifs/smb2inode.c
> +++ b/fs/cifs/smb2inode.c
> @@ -313,7 +313,7 @@ smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> rqst[num_rqst].rq_iov = close_iov;
> rqst[num_rqst].rq_nvec = 1;
> rc = SMB2_close_init(tcon, &rqst[num_rqst], COMPOUND_FID,
> - COMPOUND_FID);
> + COMPOUND_FID, false);
> smb2_set_related(&rqst[num_rqst]);
> if (rc)
> goto finished;
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 7ccbfc656478..1b581624c52b 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1206,7 +1206,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
> memset(&close_iov, 0, sizeof(close_iov));
> rqst[2].rq_iov = close_iov;
> rqst[2].rq_nvec = 1;
> - rc = SMB2_close_init(tcon, &rqst[2], COMPOUND_FID, COMPOUND_FID);
> + rc = SMB2_close_init(tcon, &rqst[2], COMPOUND_FID, COMPOUND_FID, false);
> smb2_set_related(&rqst[2]);
>
> rc = compound_send_recv(xid, ses, flags, 3, rqst,
> @@ -1361,6 +1361,45 @@ smb2_close_file(const unsigned int xid, struct cifs_tcon *tcon,
> SMB2_close(xid, tcon, fid->persistent_fid, fid->volatile_fid);
> }
>
> +static void
> +smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
> + struct cifsFileInfo *cfile)
> +{
> + struct smb2_file_network_open_info file_inf;
> + struct inode *inode;
> + int rc;
> +
> + rc = __SMB2_close(xid, tcon, cfile->fid.persistent_fid,
> + cfile->fid.volatile_fid, &file_inf);
> + if (rc)
> + return;
> +
> + inode = d_inode(cfile->dentry);
> +
> + spin_lock(&inode->i_lock);
> + CIFS_I(inode)->time = jiffies;
> +
> + /* Creation time should not need to be updated on close */
> + if (file_inf.LastWriteTime)
> + inode->i_mtime = cifs_NTtimeToUnix(file_inf.LastWriteTime);
> + if (file_inf.ChangeTime)
> + inode->i_ctime = cifs_NTtimeToUnix(file_inf.ChangeTime);
> + if (file_inf.LastAccessTime)
> + inode->i_atime = cifs_NTtimeToUnix(file_inf.LastAccessTime);
> +
> + /*
> + * i_blocks is not related to (i_size / i_blksize),
> + * but instead 512 byte (2**9) size is required for
> + * calculating num blocks.
> + */
> + if (le64_to_cpu(file_inf.AllocationSize) > 4096)
> + inode->i_blocks =
> + (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
> +
> + /* End of file and Attributes should not have to be updated on close */
> + spin_unlock(&inode->i_lock);
> +}
> +
> static int
> SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_fid, u64 volatile_fid,
> @@ -1548,7 +1587,7 @@ smb2_ioctl_query_info(const unsigned int xid,
> rqst[2].rq_iov = close_iov;
> rqst[2].rq_nvec = 1;
>
> - rc = SMB2_close_init(tcon, &rqst[2], COMPOUND_FID, COMPOUND_FID);
> + rc = SMB2_close_init(tcon, &rqst[2], COMPOUND_FID, COMPOUND_FID, false);
> if (rc)
> goto iqinf_exit;
> smb2_set_related(&rqst[2]);
> @@ -2276,7 +2315,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
> rqst[2].rq_iov = close_iov;
> rqst[2].rq_nvec = 1;
>
> - rc = SMB2_close_init(tcon, &rqst[2], COMPOUND_FID, COMPOUND_FID);
> + rc = SMB2_close_init(tcon, &rqst[2], COMPOUND_FID, COMPOUND_FID, false);
> if (rc)
> goto qic_exit;
> smb2_set_related(&rqst[2]);
> @@ -2692,7 +2731,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> rqst[2].rq_iov = close_iov;
> rqst[2].rq_nvec = 1;
>
> - rc = SMB2_close_init(tcon, &rqst[2], COMPOUND_FID, COMPOUND_FID);
> + rc = SMB2_close_init(tcon, &rqst[2], COMPOUND_FID, COMPOUND_FID, false);
> if (rc)
> goto querty_exit;
>
> @@ -4738,6 +4777,7 @@ struct smb_version_operations smb30_operations = {
> .open = smb2_open_file,
> .set_fid = smb2_set_fid,
> .close = smb2_close_file,
> + .close_getattr = smb2_close_getattr,
> .flush = smb2_flush_file,
> .async_readv = smb2_async_readv,
> .async_writev = smb2_async_writev,
> @@ -4847,6 +4887,7 @@ struct smb_version_operations smb311_operations = {
> .open = smb2_open_file,
> .set_fid = smb2_set_fid,
> .close = smb2_close_file,
> + .close_getattr = smb2_close_getattr,
> .flush = smb2_flush_file,
> .async_readv = smb2_async_readv,
> .async_writev = smb2_async_writev,
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index c095f2e6b082..6695994d2a84 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -2918,7 +2918,7 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
>
> int
> SMB2_close_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> - u64 persistent_fid, u64 volatile_fid)
> + u64 persistent_fid, u64 volatile_fid, bool query_attrs)
> {
> struct smb2_close_req *req;
> struct kvec *iov = rqst->rq_iov;
> @@ -2931,6 +2931,10 @@ SMB2_close_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
>
> req->PersistentFileId = persistent_fid;
> req->VolatileFileId = volatile_fid;
> + if (query_attrs)
> + req->Flags = SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB;
> + else
> + req->Flags = 0;
> iov[0].iov_base = (char *)req;
> iov[0].iov_len = total_len;
>
> @@ -2945,8 +2949,9 @@ SMB2_close_free(struct smb_rqst *rqst)
> }
>
> int
> -SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
> - u64 persistent_fid, u64 volatile_fid)
> +__SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
> + u64 persistent_fid, u64 volatile_fid,
> + struct smb2_file_network_open_info *pbuf)
> {
> struct smb_rqst rqst;
> struct smb2_close_rsp *rsp = NULL;
> @@ -2956,6 +2961,7 @@ SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
> int resp_buftype = CIFS_NO_BUFFER;
> int rc = 0;
> int flags = 0;
> + bool query_attrs = false;
>
> cifs_dbg(FYI, "Close\n");
>
> @@ -2970,8 +2976,13 @@ SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
> rqst.rq_iov = iov;
> rqst.rq_nvec = 1;
>
> + /* check if need to ask server to return timestamps in close response */
> + if (pbuf)
> + query_attrs = true;
> +
> trace_smb3_close_enter(xid, persistent_fid, tcon->tid, ses->Suid);
> - rc = SMB2_close_init(tcon, &rqst, persistent_fid, volatile_fid);
> + rc = SMB2_close_init(tcon, &rqst, persistent_fid, volatile_fid,
> + query_attrs);
> if (rc)
> goto close_exit;
>
> @@ -2983,14 +2994,18 @@ SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
> trace_smb3_close_err(xid, persistent_fid, tcon->tid, ses->Suid,
> rc);
> goto close_exit;
> - } else
> + } else {
> trace_smb3_close_done(xid, persistent_fid, tcon->tid,
> ses->Suid);
> + /*
> + * Note that have to subtract 4 since struct network_open_info
> + * has a final 4 byte pad that close response does not have
> + */
> + if (pbuf)
> + memcpy(pbuf, (char *)&rsp->CreationTime, sizeof(*pbuf) - 4);
> + }
>
> atomic_dec(&tcon->num_remote_opens);
> -
> - /* BB FIXME - decode close response, update inode for caching */
> -
> close_exit:
> SMB2_close_free(&rqst);
> free_rsp_buf(resp_buftype, rsp);
> @@ -3008,6 +3023,13 @@ SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
> return rc;
> }
>
> +int
> +SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
> + u64 persistent_fid, u64 volatile_fid)
> +{
> + return __SMB2_close(xid, tcon, persistent_fid, volatile_fid, NULL);
> +}
> +
> int
> smb2_validate_iov(unsigned int offset, unsigned int buffer_length,
> struct kvec *iov, unsigned int min_buf_size)
> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> index 0abfde6d0b05..63e8cf8c1075 100644
> --- a/fs/cifs/smb2pdu.h
> +++ b/fs/cifs/smb2pdu.h
> @@ -1570,6 +1570,17 @@ struct smb2_file_eof_info { /* encoding of request for level 10 */
> __le64 EndOfFile; /* new end of file value */
> } __packed; /* level 20 Set */
>
> +struct smb2_file_network_open_info {
> + __le64 CreationTime;
> + __le64 LastAccessTime;
> + __le64 LastWriteTime;
> + __le64 ChangeTime;
> + __le64 AllocationSize;
> + __le64 EndOfFile;
> + __le32 Attributes;
> + __le32 Reserved;
> +} __packed; /* level 34 Query also similar returned in close rsp and open rsp */
> +
> extern char smb2_padding[7];
>
> #endif /* _SMB2PDU_H */
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 57f7075a3587..221ce7a36439 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -154,10 +154,13 @@ extern int SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_fid, u64 volatile_fid, bool watch_tree,
> u32 completion_filter);
>
> +extern int __SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
> + u64 persistent_fid, u64 volatile_fid,
> + struct smb2_file_network_open_info *pbuf);
> extern int SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_file_id, u64 volatile_file_id);
> extern int SMB2_close_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> - u64 persistent_file_id, u64 volatile_file_id);
> + u64 persistent_fid, u64 volatile_fid, bool query_attrs);
> extern void SMB2_close_free(struct smb_rqst *rqst);
> extern int SMB2_flush(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_file_id, u64 volatile_file_id);
> --
> 2.25.1
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list