diff mbox series

[2/2] cifs: use a compound for getting an xattr

Message ID 20181106125244.28342-3-lsahlber@redhat.com (mailing list archive)
State New, archived
Headers show
Series cifs: more compounding | expand

Commit Message

Ronnie Sahlberg Nov. 6, 2018, 12:52 p.m. UTC 
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2ops.c   | 107 ++++++++++++++++++++++++++++++++++++----------------
 fs/cifs/smb2pdu.c   |  12 ------
 fs/cifs/smb2pdu.h   |   1 -
 fs/cifs/smb2proto.h |   4 --
 4 files changed, 74 insertions(+), 50 deletions(-)

Comments

Pavel Shilovsky Jan. 23, 2019, 9:31 p.m. UTC | #1 
вт, 6 нояб. 2018 г. в 04:53, Ronnie Sahlberg <lsahlber@redhat.com>:
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>

Sorry for the late review.

> ---
>  fs/cifs/smb2ops.c   | 107 ++++++++++++++++++++++++++++++++++++----------------
>  fs/cifs/smb2pdu.c   |  12 ------
>  fs/cifs/smb2pdu.h   |   1 -
>  fs/cifs/smb2proto.h |   4 --
>  4 files changed, 74 insertions(+), 50 deletions(-)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 9a952f56bbe7..d3dce3a7b84a 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -829,18 +829,47 @@ smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
>                char *ea_data, size_t buf_size,
>                struct cifs_sb_info *cifs_sb)
>  {
> -       int rc;
> -       __le16 *utf16_path;
> +       struct cifs_ses *ses = tcon->ses;
> +       __le16 *utf16_path = NULL;
> +       int flags = 0;
> +       struct smb_rqst rqst[3];
> +       int resp_buftype[3];
> +       struct kvec rsp_iov[3];
> +       struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
>         __u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
>         struct cifs_open_parms oparms;
>         struct cifs_fid fid;
> -       struct smb2_file_full_ea_info *smb2_data;
> -       int ea_buf_size = SMB2_MIN_EA_BUF;
> +       struct kvec qi_iov[1];
> +       struct smb2_file_full_ea_info *smb2_data = NULL;
> +       struct smb2_query_info_rsp *rsp = NULL;
> +       int ea_buf_size = SMB2_MAX_EA_BUF;

I don't think this will fit into 16k big buffer we allocate for
responses. With the compounding we start to consume even more space
from this buffer, so the possibility for this to fit becomes smaller.

It seems that SMB2_query_info_init() should shrink output_len to
(CIFSMaxBufSize - *all possible compounded headers + data*).

The problem was introduced before this patch but this patch makes at
easier to repro.

> +       int min_len;
> +       struct kvec close_iov[1];
> +       int rc;
>
>         utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
>         if (!utf16_path)
>                 return -ENOMEM;
>
> +       if (smb3_encryption_required(tcon))
> +               flags |= CIFS_TRANSFORM_REQ;
> +
> +       memset(rqst, 0, sizeof(rqst));
> +       resp_buftype[0] = resp_buftype[1] = resp_buftype[2] = CIFS_NO_BUFFER;
> +       memset(rsp_iov, 0, sizeof(rsp_iov));
> +
> +       smb2_data = kzalloc(ea_buf_size, GFP_KERNEL);
> +       if (smb2_data == NULL) {
> +               rc = -ENOMEM;
> +               goto qeas_exit;
> +       }
> +
> +       /* Open */
> +       memset(&open_iov, 0, sizeof(open_iov));
> +       rqst[0].rq_iov = open_iov;
> +       rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
> +
> +       memset(&oparms, 0, sizeof(oparms));
>         oparms.tcon = tcon;
>         oparms.desired_access = FILE_READ_EA;
>         oparms.disposition = FILE_OPEN;
> @@ -851,40 +880,44 @@ smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
>         oparms.fid = &fid;
>         oparms.reconnect = false;
>
> -       rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL);
> -       kfree(utf16_path);
> -       if (rc) {
> -               cifs_dbg(FYI, "open failed rc=%d\n", rc);
> -               return rc;
> -       }
> -
> -       while (1) {
> -               smb2_data = kzalloc(ea_buf_size, GFP_KERNEL);
> -               if (smb2_data == NULL) {
> -                       SMB2_close(xid, tcon, fid.persistent_fid,
> -                                  fid.volatile_fid);
> -                       return -ENOMEM;
> -               }
> +       rc = SMB2_open_init(tcon, &rqst[0], &oplock, &oparms, utf16_path);
> +       if (rc)
> +               goto qeas_exit;
> +       smb2_set_next_command(ses->server, &rqst[0]);
>
> -               rc = SMB2_query_eas(xid, tcon, fid.persistent_fid,
> -                                   fid.volatile_fid,
> -                                   ea_buf_size, smb2_data);
> +       /* Query */
> +       memset(&qi_iov, 0, sizeof(qi_iov));
> +       rqst[1].rq_iov = qi_iov;
> +       rqst[1].rq_nvec = 1;
>
> -               if (rc != -E2BIG)
> -                       break;
> +       rc = SMB2_query_info_init(tcon, &rqst[1], COMPOUND_FID, COMPOUND_FID,
> +                                 FILE_FULL_EA_INFORMATION, SMB2_O_INFO_FILE,
> +                                 0,
> +                                 ea_buf_size,
> +                                 0, NULL);
> +       if (rc)
> +               goto qeas_exit;
> +       smb2_set_next_command(ses->server, &rqst[1]);
> +       smb2_set_related(&rqst[1]);
>
> -               kfree(smb2_data);
> -               ea_buf_size <<= 1;
> +       /* Close */
> +       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);
> +       smb2_set_related(&rqst[2]);
>
> -               if (ea_buf_size > SMB2_MAX_EA_BUF) {
> -                       cifs_dbg(VFS, "EA size is too large\n");
> -                       SMB2_close(xid, tcon, fid.persistent_fid,
> -                                  fid.volatile_fid);
> -                       return -ENOMEM;
> -               }
> -       }
> +       rc = compound_send_recv(xid, ses, flags, 3, rqst,
> +                               resp_buftype, rsp_iov);
> +       if (rc)
> +               goto qeas_exit;
>
> -       SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid);
> +       min_len = sizeof(struct smb2_file_full_ea_info);
> +       rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
> +       rc = smb2_validate_and_copy_iov(le16_to_cpu(rsp->OutputBufferOffset),
> +                                       le32_to_cpu(rsp->OutputBufferLength),
> +                                       &rsp_iov[1], min_len,
> +                                       (char *)smb2_data);
>
>         /*
>          * If ea_name is NULL (listxattr) and there are no EAs, return 0 as it's
> @@ -896,7 +929,15 @@ smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
>         else if (!ea_name && rc == -ENODATA)
>                 rc = 0;
>
> + qeas_exit:
> +       kfree(utf16_path);
>         kfree(smb2_data);
> +       SMB2_open_free(&rqst[0]);
> +       SMB2_query_info_free(&rqst[1]);
> +       SMB2_close_free(&rqst[2]);
> +       free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
> +       free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
> +       free_rsp_buf(resp_buftype[2], rsp_iov[2].iov_base);
>         return rc;
>  }
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 27f86537a5d1..448031898dd4 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -2768,18 +2768,6 @@ query_info(const unsigned int xid, struct cifs_tcon *tcon,
>         return rc;
>  }
>
> -int SMB2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
> -                  u64 persistent_fid, u64 volatile_fid,
> -                  int ea_buf_size, struct smb2_file_full_ea_info *data)
> -{
> -       return query_info(xid, tcon, persistent_fid, volatile_fid,
> -                         FILE_FULL_EA_INFORMATION, SMB2_O_INFO_FILE, 0,
> -                         ea_buf_size,
> -                         sizeof(struct smb2_file_full_ea_info),
> -                         (void **)&data,
> -                         NULL);
> -}
> -
>  int SMB2_query_info(const unsigned int xid, struct cifs_tcon *tcon,
>         u64 persistent_fid, u64 volatile_fid, struct smb2_file_all_info *data)
>  {
> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> index 5671d5ee7f58..05dea6750c33 100644
> --- a/fs/cifs/smb2pdu.h
> +++ b/fs/cifs/smb2pdu.h
> @@ -1398,7 +1398,6 @@ struct smb2_file_link_info { /* encoding of request for level 11 */
>         char   FileName[0];     /* Name to be assigned to new link */
>  } __packed; /* level 11 Set */
>
> -#define SMB2_MIN_EA_BUF  2048
>  #define SMB2_MAX_EA_BUF 65536
>
>  struct smb2_file_full_ea_info { /* encoding of response for level 15 */
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 9f4e9ed9ce53..f08a094f89f4 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -153,10 +153,6 @@ extern int SMB2_close_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
>  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);
> -extern int SMB2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
> -                         u64 persistent_file_id, u64 volatile_file_id,
> -                         int ea_buf_size,
> -                         struct smb2_file_full_ea_info *data);
>  extern int SMB2_query_info(const unsigned int xid, struct cifs_tcon *tcon,
>                            u64 persistent_file_id, u64 volatile_file_id,
>                            struct smb2_file_all_info *data);
> --
> 2.13.6
>
diff mbox series

Patch

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 9a952f56bbe7..d3dce3a7b84a 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -829,18 +829,47 @@  smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
 	       char *ea_data, size_t buf_size,
 	       struct cifs_sb_info *cifs_sb)
 {
-	int rc;
-	__le16 *utf16_path;
+	struct cifs_ses *ses = tcon->ses;
+	__le16 *utf16_path = NULL;
+	int flags = 0;
+	struct smb_rqst rqst[3];
+	int resp_buftype[3];
+	struct kvec rsp_iov[3];
+	struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
 	__u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
 	struct cifs_open_parms oparms;
 	struct cifs_fid fid;
-	struct smb2_file_full_ea_info *smb2_data;
-	int ea_buf_size = SMB2_MIN_EA_BUF;
+	struct kvec qi_iov[1];
+	struct smb2_file_full_ea_info *smb2_data = NULL;
+	struct smb2_query_info_rsp *rsp = NULL;
+	int ea_buf_size = SMB2_MAX_EA_BUF;
+	int min_len;
+	struct kvec close_iov[1];
+	int rc;
 
 	utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
 	if (!utf16_path)
 		return -ENOMEM;
 
+	if (smb3_encryption_required(tcon))
+		flags |= CIFS_TRANSFORM_REQ;
+
+	memset(rqst, 0, sizeof(rqst));
+	resp_buftype[0] = resp_buftype[1] = resp_buftype[2] = CIFS_NO_BUFFER;
+	memset(rsp_iov, 0, sizeof(rsp_iov));
+
+	smb2_data = kzalloc(ea_buf_size, GFP_KERNEL);
+	if (smb2_data == NULL) {
+		rc = -ENOMEM;
+		goto qeas_exit;
+	}
+
+	/* Open */
+	memset(&open_iov, 0, sizeof(open_iov));
+	rqst[0].rq_iov = open_iov;
+	rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
+
+	memset(&oparms, 0, sizeof(oparms));
 	oparms.tcon = tcon;
 	oparms.desired_access = FILE_READ_EA;
 	oparms.disposition = FILE_OPEN;
@@ -851,40 +880,44 @@  smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
 	oparms.fid = &fid;
 	oparms.reconnect = false;
 
-	rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL);
-	kfree(utf16_path);
-	if (rc) {
-		cifs_dbg(FYI, "open failed rc=%d\n", rc);
-		return rc;
-	}
-
-	while (1) {
-		smb2_data = kzalloc(ea_buf_size, GFP_KERNEL);
-		if (smb2_data == NULL) {
-			SMB2_close(xid, tcon, fid.persistent_fid,
-				   fid.volatile_fid);
-			return -ENOMEM;
-		}
+	rc = SMB2_open_init(tcon, &rqst[0], &oplock, &oparms, utf16_path);
+	if (rc)
+		goto qeas_exit;
+	smb2_set_next_command(ses->server, &rqst[0]);
 
-		rc = SMB2_query_eas(xid, tcon, fid.persistent_fid,
-				    fid.volatile_fid,
-				    ea_buf_size, smb2_data);
+	/* Query */
+	memset(&qi_iov, 0, sizeof(qi_iov));
+	rqst[1].rq_iov = qi_iov;
+	rqst[1].rq_nvec = 1;
 
-		if (rc != -E2BIG)
-			break;
+	rc = SMB2_query_info_init(tcon, &rqst[1], COMPOUND_FID, COMPOUND_FID,
+				  FILE_FULL_EA_INFORMATION, SMB2_O_INFO_FILE,
+				  0,
+				  ea_buf_size,
+				  0, NULL);
+	if (rc)
+		goto qeas_exit;
+	smb2_set_next_command(ses->server, &rqst[1]);
+	smb2_set_related(&rqst[1]);
 
-		kfree(smb2_data);
-		ea_buf_size <<= 1;
+	/* Close */
+	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);
+	smb2_set_related(&rqst[2]);
 
-		if (ea_buf_size > SMB2_MAX_EA_BUF) {
-			cifs_dbg(VFS, "EA size is too large\n");
-			SMB2_close(xid, tcon, fid.persistent_fid,
-				   fid.volatile_fid);
-			return -ENOMEM;
-		}
-	}
+	rc = compound_send_recv(xid, ses, flags, 3, rqst,
+				resp_buftype, rsp_iov);
+	if (rc)
+		goto qeas_exit;
 
-	SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid);
+	min_len = sizeof(struct smb2_file_full_ea_info);
+	rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
+	rc = smb2_validate_and_copy_iov(le16_to_cpu(rsp->OutputBufferOffset),
+					le32_to_cpu(rsp->OutputBufferLength),
+					&rsp_iov[1], min_len,
+					(char *)smb2_data);
 
 	/*
 	 * If ea_name is NULL (listxattr) and there are no EAs, return 0 as it's
@@ -896,7 +929,15 @@  smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
 	else if (!ea_name && rc == -ENODATA)
 		rc = 0;
 
+ qeas_exit:
+	kfree(utf16_path);
 	kfree(smb2_data);
+	SMB2_open_free(&rqst[0]);
+	SMB2_query_info_free(&rqst[1]);
+	SMB2_close_free(&rqst[2]);
+	free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
+	free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
+	free_rsp_buf(resp_buftype[2], rsp_iov[2].iov_base);
 	return rc;
 }
 
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 27f86537a5d1..448031898dd4 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -2768,18 +2768,6 @@  query_info(const unsigned int xid, struct cifs_tcon *tcon,
 	return rc;
 }
 
-int SMB2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
-		   u64 persistent_fid, u64 volatile_fid,
-		   int ea_buf_size, struct smb2_file_full_ea_info *data)
-{
-	return query_info(xid, tcon, persistent_fid, volatile_fid,
-			  FILE_FULL_EA_INFORMATION, SMB2_O_INFO_FILE, 0,
-			  ea_buf_size,
-			  sizeof(struct smb2_file_full_ea_info),
-			  (void **)&data,
-			  NULL);
-}
-
 int SMB2_query_info(const unsigned int xid, struct cifs_tcon *tcon,
 	u64 persistent_fid, u64 volatile_fid, struct smb2_file_all_info *data)
 {
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 5671d5ee7f58..05dea6750c33 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -1398,7 +1398,6 @@  struct smb2_file_link_info { /* encoding of request for level 11 */
 	char   FileName[0];     /* Name to be assigned to new link */
 } __packed; /* level 11 Set */
 
-#define SMB2_MIN_EA_BUF  2048
 #define SMB2_MAX_EA_BUF 65536
 
 struct smb2_file_full_ea_info { /* encoding of response for level 15 */
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 9f4e9ed9ce53..f08a094f89f4 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -153,10 +153,6 @@  extern int SMB2_close_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
 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);
-extern int SMB2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
-			  u64 persistent_file_id, u64 volatile_file_id,
-			  int ea_buf_size,
-			  struct smb2_file_full_ea_info *data);
 extern int SMB2_query_info(const unsigned int xid, struct cifs_tcon *tcon,
 			   u64 persistent_file_id, u64 volatile_file_id,
 			   struct smb2_file_all_info *data);