reviewed-by me
On Tue, Mar 9, 2021 at 1:02 AM Paulo Alcantara <pc@xxxxxx> wrote:
>
> In case of interrupted syscalls, prevent sending CLOSE commands for
> compound CREATE+CLOSE requests by introducing an
> CIFS_CP_CREATE_CLOSE_OP flag to indicate lower layers that it should
> not send a CLOSE command to the MIDs corresponding the compound
> CREATE+CLOSE request.
>
> A simple reproducer:
>
> #!/bin/bash
>
> mount //server/share /mnt -o username=foo,password=***
> tc qdisc add dev eth0 root netem delay 450ms
> stat -f /mnt &>/dev/null & pid=$!
> sleep 0.01
> kill $pid
> tc qdisc del dev eth0 root
> umount /mnt
>
> Before patch:
>
> ...
> 6 0.256893470 192.168.122.2 → 192.168.122.15 SMB2 402 Create Request File: ;GetInfo Request FS_INFO/FileFsFullSizeInformation;Close Request
> 7 0.257144491 192.168.122.15 → 192.168.122.2 SMB2 498 Create Response File: ;GetInfo Response;Close Response
> 9 0.260798209 192.168.122.2 → 192.168.122.15 SMB2 146 Close Request File:
> 10 0.260841089 192.168.122.15 → 192.168.122.2 SMB2 130 Close Response, Error: STATUS_FILE_CLOSED
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@xxxxxx>
> ---
> fs/cifs/cifsglob.h | 19 ++++++++++---------
> fs/cifs/smb2inode.c | 1 +
> fs/cifs/smb2misc.c | 8 ++++----
> fs/cifs/smb2ops.c | 10 +++++-----
> fs/cifs/smb2proto.h | 3 +--
> fs/cifs/transport.c | 2 +-
> 6 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 3de3c5908a72..31fc8695abd6 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -257,7 +257,7 @@ struct smb_version_operations {
> /* verify the message */
> int (*check_message)(char *, unsigned int, struct TCP_Server_Info *);
> bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
> - int (*handle_cancelled_mid)(char *, struct TCP_Server_Info *);
> + int (*handle_cancelled_mid)(struct mid_q_entry *, struct TCP_Server_Info *);
> void (*downgrade_oplock)(struct TCP_Server_Info *server,
> struct cifsInodeInfo *cinode, __u32 oplock,
> unsigned int epoch, bool *purge_cache);
> @@ -1705,16 +1705,17 @@ static inline bool is_retryable_error(int error)
> #define CIFS_NO_RSP_BUF 0x040 /* no response buffer required */
>
> /* Type of request operation */
> -#define CIFS_ECHO_OP 0x080 /* echo request */
> -#define CIFS_OBREAK_OP 0x0100 /* oplock break request */
> -#define CIFS_NEG_OP 0x0200 /* negotiate request */
> +#define CIFS_ECHO_OP 0x080 /* echo request */
> +#define CIFS_OBREAK_OP 0x0100 /* oplock break request */
> +#define CIFS_NEG_OP 0x0200 /* negotiate request */
> +#define CIFS_CP_CREATE_CLOSE_OP 0x0400 /* compound create+close request */
> /* Lower bitmask values are reserved by others below. */
> -#define CIFS_SESS_OP 0x2000 /* session setup request */
> -#define CIFS_OP_MASK 0x2380 /* mask request type */
> +#define CIFS_SESS_OP 0x2000 /* session setup request */
> +#define CIFS_OP_MASK 0x2780 /* mask request type */
>
> -#define CIFS_HAS_CREDITS 0x0400 /* already has credits */
> -#define CIFS_TRANSFORM_REQ 0x0800 /* transform request before sending */
> -#define CIFS_NO_SRV_RSP 0x1000 /* there is no server response */
> +#define CIFS_HAS_CREDITS 0x0400 /* already has credits */
> +#define CIFS_TRANSFORM_REQ 0x0800 /* transform request before sending */
> +#define CIFS_NO_SRV_RSP 0x1000 /* there is no server response */
>
> /* Security Flags: indicate type of session setup needed */
> #define CIFSSEC_MAY_SIGN 0x00001
> diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
> index 1f900b81c34a..a718dc77e604 100644
> --- a/fs/cifs/smb2inode.c
> +++ b/fs/cifs/smb2inode.c
> @@ -358,6 +358,7 @@ smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> if (cfile)
> goto after_close;
> /* Close */
> + flags |= CIFS_CP_CREATE_CLOSE_OP;
> rqst[num_rqst].rq_iov = &vars->close_iov[0];
> rqst[num_rqst].rq_nvec = 1;
> rc = SMB2_close_init(tcon, server,
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index 0a55a77d94de..c99966121757 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -844,14 +844,14 @@ smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid,
> }
>
> int
> -smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server)
> +smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server)
> {
> - struct smb2_sync_hdr *sync_hdr = (struct smb2_sync_hdr *)buffer;
> - struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)buffer;
> + struct smb2_sync_hdr *sync_hdr = mid->resp_buf;
> + struct smb2_create_rsp *rsp = mid->resp_buf;
> struct cifs_tcon *tcon;
> int rc;
>
> - if (sync_hdr->Command != SMB2_CREATE ||
> + if ((mid->optype & CIFS_CP_CREATE_CLOSE_OP) || sync_hdr->Command != SMB2_CREATE ||
> sync_hdr->Status != STATUS_SUCCESS)
> return 0;
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index f5087295424c..9bae7e8deb09 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1195,7 +1195,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
> struct TCP_Server_Info *server = cifs_pick_channel(ses);
> __le16 *utf16_path = NULL;
> int ea_name_len = strlen(ea_name);
> - int flags = 0;
> + int flags = CIFS_CP_CREATE_CLOSE_OP;
> int len;
> struct smb_rqst rqst[3];
> int resp_buftype[3];
> @@ -1573,7 +1573,7 @@ smb2_ioctl_query_info(const unsigned int xid,
> struct smb_query_info qi;
> struct smb_query_info __user *pqi;
> int rc = 0;
> - int flags = 0;
> + int flags = CIFS_CP_CREATE_CLOSE_OP;
> struct smb2_query_info_rsp *qi_rsp = NULL;
> struct smb2_ioctl_rsp *io_rsp = NULL;
> void *buffer = NULL;
> @@ -2577,7 +2577,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
> {
> struct cifs_ses *ses = tcon->ses;
> struct TCP_Server_Info *server = cifs_pick_channel(ses);
> - int flags = 0;
> + int flags = CIFS_CP_CREATE_CLOSE_OP;
> struct smb_rqst rqst[3];
> int resp_buftype[3];
> struct kvec rsp_iov[3];
> @@ -2975,7 +2975,7 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> unsigned int sub_offset;
> unsigned int print_len;
> unsigned int print_offset;
> - int flags = 0;
> + int flags = CIFS_CP_CREATE_CLOSE_OP;
> struct smb_rqst rqst[3];
> int resp_buftype[3];
> struct kvec rsp_iov[3];
> @@ -3157,7 +3157,7 @@ smb2_query_reparse_tag(const unsigned int xid, struct cifs_tcon *tcon,
> struct cifs_open_parms oparms;
> struct cifs_fid fid;
> struct TCP_Server_Info *server = cifs_pick_channel(tcon->ses);
> - int flags = 0;
> + int flags = CIFS_CP_CREATE_CLOSE_OP;
> struct smb_rqst rqst[3];
> int resp_buftype[3];
> struct kvec rsp_iov[3];
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 9565e27681a5..a2eb34a8d9c9 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -246,8 +246,7 @@ extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
> extern int smb2_handle_cancelled_close(struct cifs_tcon *tcon,
> __u64 persistent_fid,
> __u64 volatile_fid);
> -extern int smb2_handle_cancelled_mid(char *buffer,
> - struct TCP_Server_Info *server);
> +extern int smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server);
> void smb2_cancelled_close_fid(struct work_struct *work);
> extern int SMB2_QFS_info(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_file_id, u64 volatile_file_id,
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index f62f512e2cb1..9438a0c35473 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -109,7 +109,7 @@ static void _cifs_mid_q_entry_release(struct kref *refcount)
> if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) &&
> midEntry->mid_state == MID_RESPONSE_RECEIVED &&
> server->ops->handle_cancelled_mid)
> - server->ops->handle_cancelled_mid(midEntry->resp_buf, server);
> + server->ops->handle_cancelled_mid(midEntry, server);
>
> midEntry->mid_state = MID_FREE;
> atomic_dec(&midCount);
> --
> 2.30.1
>
