ср, 4 дек. 2019 г. в 14:54, Ronnie Sahlberg <lsahlber@xxxxxxxxxx>:
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> ---
> fs/cifs/smb2pdu.c | 108 +++++++++++++++++++++++++++++++++++-----------------
> fs/cifs/smb2pdu.h | 2 +
> fs/cifs/smb2proto.h | 5 +++
> 3 files changed, 80 insertions(+), 35 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index ed77f94dbf1d..df903931590e 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -4214,56 +4214,36 @@ num_entries(char *bufstart, char *end_of_buf, char **lastentry, size_t size)
> /*
> * Readdir/FindFirst
> */
> -int
> -SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> - u64 persistent_fid, u64 volatile_fid, int index,
> - struct cifs_search_info *srch_inf)
> +int SMB2_query_directory_init(const unsigned int xid,
> + struct cifs_tcon *tcon, struct smb_rqst *rqst,
> + u64 persistent_fid, u64 volatile_fid,
> + int index, int info_level)
> {
> - struct smb_rqst rqst;
> + struct TCP_Server_Info *server = tcon->ses->server;
> struct smb2_query_directory_req *req;
> - struct smb2_query_directory_rsp *rsp = NULL;
> - struct kvec iov[2];
> - struct kvec rsp_iov;
> - int rc = 0;
> - int len;
> - int resp_buftype = CIFS_NO_BUFFER;
> unsigned char *bufptr;
> - struct TCP_Server_Info *server;
> - struct cifs_ses *ses = tcon->ses;
> __le16 asteriks = cpu_to_le16('*');
> - char *end_of_smb;
> unsigned int output_size = CIFSMaxBufSize;
I think we need to account for compound responses here. The response
buffer size is MAX_SMB2_HDR_SIZE + CIFSMaxBufSize, where
MAX_SMB2_HDR_SIZE is 52 transform hdr + 64 hdr + 88 create rsp. While
output_size being CIFSMaxBufSize worked for non-compounded query
directory responses it may not fit into buffer for compounded ones
when encryption is used.
It seem like we need to set it to CIFSMaxBufSize -
MAX_SMB2_CREATE_RESPONSE_SIZE - MAX_SMB2_CLOSE_RESPONSE_SIZE like to
do in smb2_query_eas().
I found other occurrences of the similar issue in smb2_query_symlink()
and smb2_ioctl_query_info(): SMB2_ioctl_init() should use
CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE -
MAX_SMB2_CLOSE_RESPONSE_SIZE too instead of CIFSMaxBufSize. Could you
fix those together with this one?
> - size_t info_buf_size;
> - int flags = 0;
> unsigned int total_len;
> -
> - if (ses && (ses->server))
> - server = ses->server;
> - else
> - return -EIO;
> + struct kvec *iov = rqst->rq_iov;
> + int len, rc;
>
> rc = smb2_plain_req_init(SMB2_QUERY_DIRECTORY, tcon, (void **) &req,
> &total_len);
> if (rc)
> return rc;
>
> - if (smb3_encryption_required(tcon))
> - flags |= CIFS_TRANSFORM_REQ;
> -
> - switch (srch_inf->info_level) {
> + switch (info_level) {
> case SMB_FIND_FILE_DIRECTORY_INFO:
> req->FileInformationClass = FILE_DIRECTORY_INFORMATION;
> - info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
> break;
> case SMB_FIND_FILE_ID_FULL_DIR_INFO:
> req->FileInformationClass = FILEID_FULL_DIRECTORY_INFORMATION;
> - info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
> break;
> default:
> cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
> - srch_inf->info_level);
> - rc = -EINVAL;
> - goto qdir_exit;
> + info_level);
> + return -EINVAL;
> }
>
> req->FileIndex = cpu_to_le32(index);
> @@ -4292,15 +4272,56 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> iov[1].iov_base = (char *)(req->Buffer);
> iov[1].iov_len = len;
>
> + trace_smb3_query_dir_enter(xid, persistent_fid, tcon->tid,
> + tcon->ses->Suid, index, output_size);
> +
> + return 0;
> +}
> +
> +void SMB2_query_directory_free(struct smb_rqst *rqst)
> +{
> + if (rqst && rqst->rq_iov) {
> + cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
> + }
> +}
> +
> +int
> +SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> + u64 persistent_fid, u64 volatile_fid, int index,
> + struct cifs_search_info *srch_inf)
> +{
> + struct smb_rqst rqst;
> + struct kvec iov[SMB2_QUERY_DIRECTORY_IOV_SIZE];
> + struct smb2_query_directory_rsp *rsp = NULL;
> + int resp_buftype = CIFS_NO_BUFFER;
> + struct kvec rsp_iov;
> + int rc = 0;
> + struct TCP_Server_Info *server;
> + struct cifs_ses *ses = tcon->ses;
> + char *end_of_smb;
> + size_t info_buf_size;
> + int flags = 0;
> +
> + if (ses && (ses->server))
> + server = ses->server;
> + else
> + return -EIO;
> +
> + if (smb3_encryption_required(tcon))
> + flags |= CIFS_TRANSFORM_REQ;
> +
> memset(&rqst, 0, sizeof(struct smb_rqst));
> + memset(&iov, 0, sizeof(iov));
> rqst.rq_iov = iov;
> - rqst.rq_nvec = 2;
> + rqst.rq_nvec = SMB2_QUERY_DIRECTORY_IOV_SIZE;
>
> - trace_smb3_query_dir_enter(xid, persistent_fid, tcon->tid,
> - tcon->ses->Suid, index, output_size);
> + rc = SMB2_query_directory_init(xid, tcon, &rqst, persistent_fid,
> + volatile_fid, index,
> + srch_inf->info_level);
> + if (rc)
> + goto qdir_exit;
>
> rc = cifs_send_recv(xid, ses, &rqst, &resp_buftype, flags, &rsp_iov);
> - cifs_small_buf_release(req);
> rsp = (struct smb2_query_directory_rsp *)rsp_iov.iov_base;
>
> if (rc) {
> @@ -4318,6 +4339,20 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> goto qdir_exit;
> }
>
> + switch (srch_inf->info_level) {
> + case SMB_FIND_FILE_DIRECTORY_INFO:
> + info_buf_size = sizeof(FILE_DIRECTORY_INFO) - 1;
> + break;
> + case SMB_FIND_FILE_ID_FULL_DIR_INFO:
> + info_buf_size = sizeof(SEARCH_ID_FULL_DIR_INFO) - 1;
> + break;
> + default:
> + cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
> + srch_inf->info_level);
> + rc = -EINVAL;
> + goto qdir_exit;
> + }
> +
> rc = smb2_validate_iov(le16_to_cpu(rsp->OutputBufferOffset),
> le32_to_cpu(rsp->OutputBufferLength), &rsp_iov,
> info_buf_size);
> @@ -4353,11 +4388,14 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> else
> cifs_tcon_dbg(VFS, "illegal search buffer type\n");
>
> + rsp = NULL;
minor: the subsequent patch removes this assignment, probably better
to not do it here.
> + resp_buftype = CIFS_NO_BUFFER;
> +
> trace_smb3_query_dir_done(xid, persistent_fid, tcon->tid,
> tcon->ses->Suid, index, srch_inf->entries_in_buffer);
> - return rc;
>
> qdir_exit:
> + SMB2_query_directory_free(&rqst);
> free_rsp_buf(resp_buftype, rsp);
> return rc;
> }
> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> index f264e1d36fe1..caf323be0d7f 100644
> --- a/fs/cifs/smb2pdu.h
> +++ b/fs/cifs/smb2pdu.h
> @@ -1272,6 +1272,8 @@ struct smb2_echo_rsp {
> #define SMB2_INDEX_SPECIFIED 0x04
> #define SMB2_REOPEN 0x10
>
> +#define SMB2_QUERY_DIRECTORY_IOV_SIZE 2
> +
> struct smb2_query_directory_req {
> struct smb2_sync_hdr sync_hdr;
> __le16 StructureSize; /* Must be 33 */
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index d21a5fcc8d06..ba48ce9af620 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -194,6 +194,11 @@ extern int SMB2_echo(struct TCP_Server_Info *server);
> extern int SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_fid, u64 volatile_fid, int index,
> struct cifs_search_info *srch_inf);
> +extern int SMB2_query_directory_init(unsigned int xid, struct cifs_tcon *tcon,
> + struct smb_rqst *rqst,
> + u64 persistent_fid, u64 volatile_fid,
> + int index, int info_level);
> +extern void SMB2_query_directory_free(struct smb_rqst *rqst);
> extern int SMB2_set_eof(const unsigned int xid, struct cifs_tcon *tcon,
> u64 persistent_fid, u64 volatile_fid, u32 pid,
> __le64 *eof);
> --
> 2.13.6
>
Other than the note about buffer sizes above the series looks good at
the first glance.
--
Best regards,
Pavel Shilovsky
