| Message ID | 20180413053723.28148-2-lsahlber@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2018-04-12 22:37 GMT-07:00 Ronnie Sahlberg <lsahlber@redhat.com>: > Separate out all the 4 byte rfc1002 headers so that they are no longer > part of the SMB2 header structures to prepare for future work to add > compounding support. Thanks for making the patch smaller! Please find my comments below. > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/connect.c | 6 ++- > fs/cifs/smb2misc.c | 88 ++++++++++++++++-------------------- > fs/cifs/smb2ops.c | 115 ++++++++++++++++++++++++++++-------------------- > fs/cifs/smb2pdu.c | 22 ++++----- > fs/cifs/smb2pdu.h | 26 ++--------- > fs/cifs/smb2proto.h | 3 +- > fs/cifs/smb2transport.c | 12 +++-- > 7 files changed, 129 insertions(+), 143 deletions(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index e8830f076a7f..ea426bb97872 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -874,7 +874,11 @@ cifs_demultiplex_thread(void *p) > length = cifs_read_from_socket(server, buf, pdu_length); > if (length < 0) > continue; > - server->total_read = length; > + > + if (server->vals->header_preamble_size == 0) > + server->total_read = 0; > + else > + server->total_read = length; > > /* > * The right amount was read from socket - 4 bytes, > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > index 68ea8491c160..f73bcc825002 100644 > --- a/fs/cifs/smb2misc.c > +++ b/fs/cifs/smb2misc.c > @@ -131,25 +131,20 @@ static __u32 get_neg_ctxt_len(struct smb2_hdr *hdr, __u32 len, __u32 non_ctxlen, > #endif /* CIFS_SMB311 */ > > int > -smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) > +smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *srvr) > { > - struct smb2_pdu *pdu = (struct smb2_pdu *)buf; > - struct smb2_hdr *hdr = &pdu->hdr; > - struct smb2_sync_hdr *shdr = get_sync_hdr(buf); > + struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)buf; Until we remove get_sync_hdr() this is not necessary. The whole logic of introducing get_sync_hdr() function was to abstract getting sync header regardless of the underneath changes. > + struct smb2_sync_pdu *pdu = (struct smb2_sync_pdu *)shdr; > __u64 mid; > - __u32 len = get_rfc1002_length(buf); I would suggest to not rename variables here: assign len to srvr->pdu_size instead and leave length variable as is. > __u32 clc_len; /* calculated length */ > int command; > - > - /* BB disable following printk later */ > - cifs_dbg(FYI, "%s length: 0x%x, smb_buf_length: 0x%x\n", > - __func__, length, len); > + int pdu_size = sizeof(struct smb2_sync_pdu); > + int hdr_size = sizeof(struct smb2_sync_hdr); > > /* > * Add function to do table lookup of StructureSize by command > * ie Validate the wct via smb2_struct_sizes table above > */ > - > if (shdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM) { > struct smb2_transform_hdr *thdr = > (struct smb2_transform_hdr *)buf; > @@ -173,8 +168,8 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) > } > > mid = le64_to_cpu(shdr->MessageId); > - if (length < sizeof(struct smb2_pdu)) { > - if ((length >= sizeof(struct smb2_hdr)) > + if (len < pdu_size) { > + if ((len >= hdr_size) use length variable and sizeofs of appropriate structures here. pdu_size and hdr_size variables don't seem to be needed. > && (shdr->Status != 0)) { > pdu->StructureSize2 = 0; > /* > @@ -227,13 +222,7 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) > } > } > > - if (srvr->vals->header_preamble_size + len != length) { > - cifs_dbg(VFS, "Total length %u RFC1002 length %zu mismatch mid %llu\n", > - length, srvr->vals->header_preamble_size + len, mid); > - return 1; > - } if we leave length variable and len = srvr->pdu_size this check can be left as it. > - > - clc_len = smb2_calc_size(hdr); > + clc_len = smb2_calc_size(buf); > > #ifdef CONFIG_CIFS_SMB311 > if (shdr->Command == SMB2_NEGOTIATE) > @@ -305,15 +294,14 @@ static const bool has_smb2_data_area[NUMBER_OF_SMB2_COMMANDS] = { > * area and the offset to it (from the beginning of the smb are also returned. > */ > char * > -smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *hdr) > +smb2_get_data_area_len(int *off, int *len, struct smb2_sync_hdr *shdr) > { > - struct smb2_sync_hdr *shdr = get_sync_hdr(hdr); the same: until get_sync_hdr() is removed, we can leave this as is. > *off = 0; > *len = 0; > > /* error responses do not have data area */ > if (shdr->Status && shdr->Status != STATUS_MORE_PROCESSING_REQUIRED && > - (((struct smb2_err_rsp *)hdr)->StructureSize) == > + (((struct smb2_err_rsp *)shdr)->StructureSize) == > SMB2_ERROR_STRUCTURE_SIZE2) > return NULL; > > @@ -325,42 +313,44 @@ smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *hdr) > switch (shdr->Command) { > case SMB2_NEGOTIATE: > *off = le16_to_cpu( > - ((struct smb2_negotiate_rsp *)hdr)->SecurityBufferOffset); > + ((struct smb2_sess_setup_rsp *)shdr)->SecurityBufferOffset); > *len = le16_to_cpu( > - ((struct smb2_negotiate_rsp *)hdr)->SecurityBufferLength); > + ((struct smb2_sess_setup_rsp *)shdr)->SecurityBufferLength); struct smb2_negotiate_rsp instead of struct smb2_sess_setup_rsp. > break; > case SMB2_SESSION_SETUP: > *off = le16_to_cpu( > - ((struct smb2_sess_setup_rsp *)hdr)->SecurityBufferOffset); > + ((struct smb2_sess_setup_rsp *)shdr)->SecurityBufferOffset); > *len = le16_to_cpu( > - ((struct smb2_sess_setup_rsp *)hdr)->SecurityBufferLength); > + ((struct smb2_sess_setup_rsp *)shdr)->SecurityBufferLength); > break; > case SMB2_CREATE: > *off = le32_to_cpu( > - ((struct smb2_create_rsp *)hdr)->CreateContextsOffset); > + ((struct smb2_create_rsp *)shdr)->CreateContextsOffset); > *len = le32_to_cpu( > - ((struct smb2_create_rsp *)hdr)->CreateContextsLength); > + ((struct smb2_create_rsp *)shdr)->CreateContextsLength); > break; > case SMB2_QUERY_INFO: > *off = le16_to_cpu( > - ((struct smb2_query_info_rsp *)hdr)->OutputBufferOffset); > + ((struct smb2_query_info_rsp *)shdr)->OutputBufferOffset); > *len = le32_to_cpu( > - ((struct smb2_query_info_rsp *)hdr)->OutputBufferLength); > + ((struct smb2_query_info_rsp *)shdr)->OutputBufferLength); > break; > case SMB2_READ: > - *off = ((struct smb2_read_rsp *)hdr)->DataOffset; > - *len = le32_to_cpu(((struct smb2_read_rsp *)hdr)->DataLength); > + /* TODO: is this a bug ? */ Why might it be a bug? > + *off = ((struct smb2_read_rsp *)shdr)->DataOffset; > + *len = le32_to_cpu(((struct smb2_read_rsp *)shdr)->DataLength); > break; > case SMB2_QUERY_DIRECTORY: > *off = le16_to_cpu( > - ((struct smb2_query_directory_rsp *)hdr)->OutputBufferOffset); > + ((struct smb2_query_directory_rsp *)shdr)->OutputBufferOffset); > *len = le32_to_cpu( > - ((struct smb2_query_directory_rsp *)hdr)->OutputBufferLength); > + ((struct smb2_query_directory_rsp *)shdr)->OutputBufferLength); > break; > case SMB2_IOCTL: > *off = le32_to_cpu( > - ((struct smb2_ioctl_rsp *)hdr)->OutputOffset); > - *len = le32_to_cpu(((struct smb2_ioctl_rsp *)hdr)->OutputCount); > + ((struct smb2_ioctl_rsp *)shdr)->OutputOffset); > + *len = le32_to_cpu( > + ((struct smb2_ioctl_rsp *)shdr)->OutputCount); > break; > case SMB2_CHANGE_NOTIFY: > default: > @@ -405,13 +395,12 @@ smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *hdr) > unsigned int > smb2_calc_size(void *buf) > { > - struct smb2_pdu *pdu = (struct smb2_pdu *)buf; > - struct smb2_hdr *hdr = &pdu->hdr; > - struct smb2_sync_hdr *shdr = get_sync_hdr(hdr); > + struct smb2_sync_pdu *pdu = (struct smb2_sync_pdu *)buf; the same logic of keeping get_sync_hdr(). > + struct smb2_sync_hdr *shdr = &pdu->sync_hdr; > int offset; /* the offset from the beginning of SMB to data area */ > int data_length; /* the length of the variable length data area */ > /* Structure Size has already been checked to make sure it is 64 */ > - int len = 4 + le16_to_cpu(shdr->StructureSize); > + int len = le16_to_cpu(shdr->StructureSize); > > /* > * StructureSize2, ie length of fixed parameter area has already > @@ -422,7 +411,7 @@ smb2_calc_size(void *buf) > if (has_smb2_data_area[le16_to_cpu(shdr->Command)] == false) > goto calc_size_exit; > > - smb2_get_data_area_len(&offset, &data_length, hdr); > + smb2_get_data_area_len(&offset, &data_length, shdr); > cifs_dbg(FYI, "SMB2 data length %d offset %d\n", data_length, offset); > > if (data_length > 0) { > @@ -430,15 +419,14 @@ smb2_calc_size(void *buf) > * Check to make sure that data area begins after fixed area, > * Note that last byte of the fixed area is part of data area > * for some commands, typically those with odd StructureSize, > - * so we must add one to the calculation (and 4 to account for > - * the size of the RFC1001 hdr. > + * so we must add one to the calculation. > */ > - if (offset + 4 + 1 < len) { > + if (offset + 1 < len) { > cifs_dbg(VFS, "data area offset %d overlaps SMB2 header %d\n", > - offset + 4 + 1, len); > + offset + 1, len); > data_length = 0; > } else { > - len = 4 + offset + data_length; > + len = offset + data_length; > } > } > calc_size_exit: > @@ -621,7 +609,7 @@ smb2_is_valid_lease_break(char *buffer) > bool > smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) > { > - struct smb2_oplock_break_rsp *rsp = (struct smb2_oplock_break_rsp *)buffer; > + struct smb2_oplock_break *rsp = (struct smb2_oplock_break *)buffer; > struct list_head *tmp, *tmp1, *tmp2; > struct cifs_ses *ses; > struct cifs_tcon *tcon; > @@ -630,7 +618,7 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) > > cifs_dbg(FYI, "Checking for oplock break\n"); > > - if (rsp->hdr.sync_hdr.Command != SMB2_OPLOCK_BREAK) > + if (rsp->sync_hdr.Command != SMB2_OPLOCK_BREAK) > return false; > > if (rsp->StructureSize != > @@ -721,8 +709,8 @@ smb2_cancelled_close_fid(struct work_struct *work) > int > smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server) > { > - struct smb2_sync_hdr *sync_hdr = get_sync_hdr(buffer); > - struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)buffer; > + struct smb2_sync_hdr *sync_hdr = (struct smb2_sync_hdr *)buffer; > + struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)sync_hdr; > struct cifs_tcon *tcon; > struct close_cancelled_open *cancelled; > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index b4ae932ea134..945a7b1ed8d7 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -123,7 +123,8 @@ smb2_get_credits_field(struct TCP_Server_Info *server, const int optype) > static unsigned int > smb2_get_credits(struct mid_q_entry *mid) > { > - struct smb2_sync_hdr *shdr = get_sync_hdr(mid->resp_buf); > + char *buf = mid->resp_buf; > + struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)buf; buf variable is not needed here, mid->rsp_buf can be used instead. > > return le16_to_cpu(shdr->CreditRequest); > } > @@ -2054,12 +2055,11 @@ smb2_dir_needs_close(struct cifsFileInfo *cfile) > } > > static void > -fill_transform_hdr(struct TCP_Server_Info *server, > - struct smb2_transform_hdr *tr_hdr, struct smb_rqst *old_rq) > +fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len, > + struct smb_rqst *old_rq) > { > struct smb2_sync_hdr *shdr = > (struct smb2_sync_hdr *)old_rq->rq_iov[1].iov_base; > - unsigned int orig_len = get_rfc1002_length(old_rq->rq_iov[0].iov_base); Since this patch involves many manipulations with transform header, is it possible to do this as a preparation step? E.g. remove RFC header from transform header only and make encryption code use a separate iov for this header by having the following abstraction iov[0].iov_base = buf, iov[0].iov_len = header_preamble_size, iov[1].iov_base = buf + header_preamble_size, iov[1].iov_len = buf_size - header_preamble_size. The subsequent patch will zero header_preamble_size value moving the code to the required state. > > memset(tr_hdr, 0, sizeof(struct smb2_transform_hdr)); > tr_hdr->ProtocolId = SMB2_TRANSFORM_PROTO_NUM; > @@ -2067,8 +2067,6 @@ fill_transform_hdr(struct TCP_Server_Info *server, > tr_hdr->Flags = cpu_to_le16(0x01); > get_random_bytes(&tr_hdr->Nonce, SMB3_AES128CMM_NONCE); > memcpy(&tr_hdr->SessionId, &shdr->SessionId, 8); > - inc_rfc1001_len(tr_hdr, sizeof(struct smb2_transform_hdr) - server->vals->header_preamble_size); > - inc_rfc1001_len(tr_hdr, orig_len); > } > > /* We can not use the normal sg_set_buf() as we will sometimes pass a > @@ -2080,11 +2078,16 @@ static inline void smb2_sg_set_buf(struct scatterlist *sg, const void *buf, > sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf)); > } > > +/* Assumes: > + * rqst->rq_iov[0] is rfc1002 length > + * rqst->rq_iov[1] is tranform header > + * rqst->rq_iov[2+] data to be encrypted/decrypted > + */ > static struct scatterlist * > init_sg(struct smb_rqst *rqst, u8 *sign) > { > - unsigned int sg_len = rqst->rq_nvec + rqst->rq_npages + 1; > - unsigned int assoc_data_len = sizeof(struct smb2_transform_hdr) - 24; > + unsigned int sg_len = rqst->rq_nvec + rqst->rq_npages; > + unsigned int assoc_data_len = sizeof(struct smb2_transform_hdr) - 20; > struct scatterlist *sg; > unsigned int i; > unsigned int j; > @@ -2094,10 +2097,10 @@ init_sg(struct smb_rqst *rqst, u8 *sign) > return NULL; > > sg_init_table(sg, sg_len); > - smb2_sg_set_buf(&sg[0], rqst->rq_iov[0].iov_base + 24, assoc_data_len); > - for (i = 1; i < rqst->rq_nvec; i++) > - smb2_sg_set_buf(&sg[i], rqst->rq_iov[i].iov_base, > - rqst->rq_iov[i].iov_len); > + smb2_sg_set_buf(&sg[0], rqst->rq_iov[1].iov_base + 20, assoc_data_len); > + for (i = 1; i < rqst->rq_nvec - 1; i++) > + smb2_sg_set_buf(&sg[i], rqst->rq_iov[i+1].iov_base, > + rqst->rq_iov[i+1].iov_len); > for (j = 0; i < sg_len - 1; i++, j++) { > unsigned int len = (j < rqst->rq_npages - 1) ? rqst->rq_pagesz > : rqst->rq_tailsz; > @@ -2129,9 +2132,10 @@ smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key) > } > /* > * Encrypt or decrypt @rqst message. @rqst has the following format: > - * iov[0] - transform header (associate data), > - * iov[1-N] and pages - data to encrypt. > - * On success return encrypted data in iov[1-N] and pages, leave iov[0] > + * iov[0] - rfc1002 length > + * iov[1] - transform header (associate data), > + * iov[2-N] and pages - data to encrypt. > + * On success return encrypted data in iov[2-N] and pages, leave iov[0-1] > * untouched. > */ > static int > @@ -2226,6 +2230,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc) > return rc; > } > > +/* > + * This is called from smb_send_rqst. At this point we have the rfc1002 > + * header as the first element in the vector. > + */ > static int > smb3_init_transform_rq(struct TCP_Server_Info *server, struct smb_rqst *new_rq, > struct smb_rqst *old_rq) > @@ -2234,6 +2242,7 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, struct smb_rqst *new_rq, > struct page **pages; > struct smb2_transform_hdr *tr_hdr; > unsigned int npages = old_rq->rq_npages; > + unsigned int orig_len = get_rfc1002_length(old_rq->rq_iov[0].iov_base); > int i; > int rc = -ENOMEM; > > @@ -2252,24 +2261,34 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, struct smb_rqst *new_rq, > goto err_free_pages; > } > > - iov = kmalloc_array(old_rq->rq_nvec, sizeof(struct kvec), GFP_KERNEL); > + /* Make space for one extra iov to hold the transform header */ > + iov = kmalloc_array(old_rq->rq_nvec + 1, sizeof(struct kvec), > + GFP_KERNEL); > if (!iov) > goto err_free_pages; > > /* copy all iovs from the old except the 1st one (rfc1002 length) */ > - memcpy(&iov[1], &old_rq->rq_iov[1], > + memcpy(&iov[2], &old_rq->rq_iov[1], > sizeof(struct kvec) * (old_rq->rq_nvec - 1)); > + /* copy the rfc1002 iov */ > + iov[0].iov_base = old_rq->rq_iov[0].iov_base; > + iov[0].iov_len = old_rq->rq_iov[0].iov_len; > + > new_rq->rq_iov = iov; > - new_rq->rq_nvec = old_rq->rq_nvec; > + new_rq->rq_nvec = old_rq->rq_nvec + 1; > > tr_hdr = kmalloc(sizeof(struct smb2_transform_hdr), GFP_KERNEL); > if (!tr_hdr) > goto err_free_iov; > > - /* fill the 1st iov with a transform header */ > - fill_transform_hdr(server, tr_hdr, old_rq); > - new_rq->rq_iov[0].iov_base = tr_hdr; > - new_rq->rq_iov[0].iov_len = sizeof(struct smb2_transform_hdr); > + /* fill the 2nd iov with a transform header */ > + fill_transform_hdr(tr_hdr, orig_len, old_rq); > + new_rq->rq_iov[1].iov_base = tr_hdr; > + new_rq->rq_iov[1].iov_len = sizeof(struct smb2_transform_hdr); > + > + /* Update rfc1002 header */ > + inc_rfc1001_len(new_rq->rq_iov[0].iov_base, > + sizeof(struct smb2_transform_hdr)); > > /* copy pages form the old */ > for (i = 0; i < npages; i++) { > @@ -2309,7 +2328,7 @@ smb3_free_transform_rq(struct smb_rqst *rqst) > put_page(rqst->rq_pages[i]); > kfree(rqst->rq_pages); > /* free transform header */ > - kfree(rqst->rq_iov[0].iov_base); > + kfree(rqst->rq_iov[1].iov_base); > kfree(rqst->rq_iov); > } > > @@ -2326,18 +2345,19 @@ decrypt_raw_data(struct TCP_Server_Info *server, char *buf, > unsigned int buf_data_size, struct page **pages, > unsigned int npages, unsigned int page_data_size) > { > - struct kvec iov[2]; > + struct kvec iov[3]; > struct smb_rqst rqst = {NULL}; > - struct smb2_hdr *hdr; > int rc; > > - iov[0].iov_base = buf; > - iov[0].iov_len = sizeof(struct smb2_transform_hdr); > - iov[1].iov_base = buf + sizeof(struct smb2_transform_hdr); > - iov[1].iov_len = buf_data_size; > + iov[0].iov_base = NULL; > + iov[0].iov_len = 0; > + iov[1].iov_base = buf; > + iov[1].iov_len = sizeof(struct smb2_transform_hdr); > + iov[2].iov_base = buf + sizeof(struct smb2_transform_hdr); > + iov[2].iov_len = buf_data_size; > > rqst.rq_iov = iov; > - rqst.rq_nvec = 2; > + rqst.rq_nvec = 3; > rqst.rq_pages = pages; > rqst.rq_npages = npages; > rqst.rq_pagesz = PAGE_SIZE; > @@ -2349,10 +2369,9 @@ decrypt_raw_data(struct TCP_Server_Info *server, char *buf, > if (rc) > return rc; > > - memmove(buf + server->vals->header_preamble_size, iov[1].iov_base, buf_data_size); > - hdr = (struct smb2_hdr *)buf; > - hdr->smb2_buf_length = cpu_to_be32(buf_data_size + page_data_size); > - server->total_read = buf_data_size + page_data_size + server->vals->header_preamble_size; > + memmove(buf + server->vals->header_preamble_size, iov[2].iov_base, buf_data_size); > + > + server->total_read = buf_data_size + page_data_size; > > return rc; > } > @@ -3106,8 +3125,8 @@ struct smb_version_values smb20_values = { > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, > - .header_size = sizeof(struct smb2_hdr), > - .header_preamble_size = 4, > + .header_size = sizeof(struct smb2_sync_hdr), > + .header_preamble_size = 0, > .max_header_size = MAX_SMB2_HDR_SIZE, > .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, > .lock_cmd = SMB2_LOCK, > @@ -3127,8 +3146,8 @@ struct smb_version_values smb21_values = { > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, > - .header_size = sizeof(struct smb2_hdr), > - .header_preamble_size = 4, > + .header_size = sizeof(struct smb2_sync_hdr), > + .header_preamble_size = 0, > .max_header_size = MAX_SMB2_HDR_SIZE, > .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, > .lock_cmd = SMB2_LOCK, > @@ -3148,8 +3167,8 @@ struct smb_version_values smb3any_values = { > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, > - .header_size = sizeof(struct smb2_hdr), > - .header_preamble_size = 4, > + .header_size = sizeof(struct smb2_sync_hdr), > + .header_preamble_size = 0, > .max_header_size = MAX_SMB2_HDR_SIZE, > .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, > .lock_cmd = SMB2_LOCK, > @@ -3169,8 +3188,8 @@ struct smb_version_values smbdefault_values = { > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, > - .header_size = sizeof(struct smb2_hdr), > - .header_preamble_size = 4, > + .header_size = sizeof(struct smb2_sync_hdr), > + .header_preamble_size = 0, > .max_header_size = MAX_SMB2_HDR_SIZE, > .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, > .lock_cmd = SMB2_LOCK, > @@ -3190,8 +3209,8 @@ struct smb_version_values smb30_values = { > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, > - .header_size = sizeof(struct smb2_hdr), > - .header_preamble_size = 4, > + .header_size = sizeof(struct smb2_sync_hdr), > + .header_preamble_size = 0, > .max_header_size = MAX_SMB2_HDR_SIZE, > .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, > .lock_cmd = SMB2_LOCK, > @@ -3211,8 +3230,8 @@ struct smb_version_values smb302_values = { > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, > - .header_size = sizeof(struct smb2_hdr), > - .header_preamble_size = 4, > + .header_size = sizeof(struct smb2_sync_hdr), > + .header_preamble_size = 0, > .max_header_size = MAX_SMB2_HDR_SIZE, > .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, > .lock_cmd = SMB2_LOCK, > @@ -3233,8 +3252,8 @@ struct smb_version_values smb311_values = { > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, > - .header_size = sizeof(struct smb2_hdr), > - .header_preamble_size = 4, > + .header_size = sizeof(struct smb2_sync_hdr), > + .header_preamble_size = 0, > .max_header_size = MAX_SMB2_HDR_SIZE, > .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, > .lock_cmd = SMB2_LOCK, > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index d7f394c41c0d..be18e653ecca 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -689,7 +689,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses) > server->capabilities |= SMB2_NT_FIND | SMB2_LARGE_FILES; > > security_blob = smb2_get_data_area_len(&blob_offset, &blob_length, > - &rsp->hdr); > + get_sync_hdr(&rsp->hdr)); With RFC header removed, both rsp->hdr and get_sync_hdr(&rsp_hdr) are the same pointers. > /* > * See MS-SMB2 section 2.2.4: if no blob, client picks default which > * for us will be > @@ -1990,7 +1990,6 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, > { > struct smb2_ioctl_req *req; > struct smb2_ioctl_rsp *rsp; > - struct smb2_sync_hdr *shdr; > struct cifs_ses *ses; > struct kvec iov[2]; > struct kvec rsp_iov; > @@ -2111,7 +2110,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, > goto ioctl_exit; > } > > - if (get_rfc1002_length(rsp) < le32_to_cpu(rsp->OutputOffset) + *plen) { > + if (rsp_iov.iov_len < le32_to_cpu(rsp->OutputOffset) + *plen) { rsp_iov.iov_len - preamble_header_size can be used instead (separate patch). > cifs_dbg(VFS, "Malformed ioctl resp: len %d offset %d\n", *plen, > le32_to_cpu(rsp->OutputOffset)); > *plen = 0; > @@ -2125,8 +2124,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, > goto ioctl_exit; > } > > - shdr = get_sync_hdr(rsp); > - memcpy(*out_data, (char *)shdr + le32_to_cpu(rsp->OutputOffset), *plen); > + memcpy(*out_data, (char *)rsp + le32_to_cpu(rsp->OutputOffset), *plen); the same logic for get_sync_hdr(). > ioctl_exit: > free_rsp_buf(resp_buftype, rsp); > return rc; > @@ -2800,7 +2798,6 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms, > int resp_buftype, rc = -EACCES; > struct smb2_read_plain_req *req = NULL; > struct smb2_read_rsp *rsp = NULL; > - struct smb2_sync_hdr *shdr; > struct kvec iov[1]; > struct kvec rsp_iov; > unsigned int total_len; > @@ -2841,10 +2838,8 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms, > *nbytes = 0; > } > > - shdr = get_sync_hdr(rsp); the same logic for get_sync_hdr(). > - > if (*buf) { > - memcpy(*buf, (char *)shdr + rsp->DataOffset, *nbytes); > + memcpy(*buf, (char *)rsp + rsp->DataOffset, *nbytes); > free_rsp_buf(resp_buftype, rsp_iov.iov_base); > } else if (resp_buftype != CIFS_NO_BUFFER) { > *buf = rsp_iov.iov_base; > @@ -3271,10 +3266,9 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon, > cifs_buf_release(srch_inf->ntwrk_buf_start); > } > srch_inf->ntwrk_buf_start = (char *)rsp; > - srch_inf->srch_entries_start = srch_inf->last_entry = 4 /* rfclen */ + > - (char *)&rsp->hdr + le16_to_cpu(rsp->OutputBufferOffset); > - /* 4 for rfc1002 length field */ > - end_of_smb = get_rfc1002_length(rsp) + 4 + (char *)&rsp->hdr; > + srch_inf->srch_entries_start = srch_inf->last_entry = > + (char *)rsp + le16_to_cpu(rsp->OutputBufferOffset); please use premable_header_size instead of "+4" > + end_of_smb = rsp_iov.iov_len + (char *)rsp; and rsp_iov.iov_len - preamble_header_size + (char *)rsp above as a preparation and move to a separate patch. > srch_inf->entries_in_buffer = > num_entries(srch_inf->srch_entries_start, end_of_smb, > &srch_inf->last_entry, info_buf_size); > @@ -3510,7 +3504,7 @@ SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon, > __u8 oplock_level) > { > int rc; > - struct smb2_oplock_break_req *req = NULL; > + struct smb2_oplock_break *req = NULL; > struct cifs_ses *ses = tcon->ses; > int flags = CIFS_OBREAK_OP; > unsigned int total_len; > diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h > index 6093e5142b2b..d8d9eb73e216 100644 > --- a/fs/cifs/smb2pdu.h > +++ b/fs/cifs/smb2pdu.h > @@ -123,24 +123,18 @@ struct smb2_sync_pdu { > } __packed; > > struct smb2_hdr { > - __be32 smb2_buf_length; /* big endian on wire */ > - /* length is only two or three bytes - with */ > - /* one or two byte type preceding it that MBZ */ > - struct smb2_sync_hdr sync_hdr; > + struct smb2_sync_hdr sync_hdr; > } __packed; > > struct smb2_pdu { > - struct smb2_hdr hdr; > - __le16 StructureSize2; /* size of wct area (varies, request specific) */ > + struct smb2_hdr hdr; > + __le16 StructureSize2; /* size of wct area (varies, request specific) */ > } __packed; > > #define SMB3_AES128CMM_NONCE 11 > #define SMB3_AES128GCM_NONCE 12 > > struct smb2_transform_hdr { > - __be32 smb2_buf_length; /* big endian on wire */ > - /* length is only two or three bytes - with > - one or two byte type preceding it that MBZ */ > __le32 ProtocolId; /* 0xFD 'S' 'M' 'B' */ > __u8 Signature[16]; > __u8 Nonce[16]; > @@ -1154,8 +1148,7 @@ struct smb2_set_info_rsp { > __le16 StructureSize; /* Must be 2 */ > } __packed; > > -/* oplock break without an rfc1002 header */ > -struct smb2_oplock_break_req { > +struct smb2_oplock_break { > struct smb2_sync_hdr sync_hdr; > __le16 StructureSize; /* Must be 24 */ > __u8 OplockLevel; > @@ -1165,17 +1158,6 @@ struct smb2_oplock_break_req { > __u64 VolatileFid; > } __packed; > > -/* oplock break with an rfc1002 header */ > -struct smb2_oplock_break_rsp { > - struct smb2_hdr hdr; > - __le16 StructureSize; /* Must be 24 */ > - __u8 OplockLevel; > - __u8 Reserved; > - __le32 Reserved2; > - __u64 PersistentFid; > - __u64 VolatileFid; > -} __packed; > - > #define SMB2_NOTIFY_BREAK_LEASE_FLAG_ACK_REQUIRED cpu_to_le32(0x01) > > struct smb2_lease_break { > diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h > index 8ba24a95db71..30cdc0aa4e8e 100644 > --- a/fs/cifs/smb2proto.h > +++ b/fs/cifs/smb2proto.h > @@ -37,7 +37,8 @@ extern int map_smb2_to_linux_error(char *buf, bool log_err); > extern int smb2_check_message(char *buf, unsigned int length, > struct TCP_Server_Info *server); > extern unsigned int smb2_calc_size(void *buf); > -extern char *smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *hdr); > +extern char *smb2_get_data_area_len(int *off, int *len, > + struct smb2_sync_hdr *shdr); > extern __le16 *cifs_convert_path_to_utf16(const char *from, > struct cifs_sb_info *cifs_sb); > > diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c > index 8806f3f76c1d..8372a0cecdb0 100644 > --- a/fs/cifs/smb2transport.c > +++ b/fs/cifs/smb2transport.c > @@ -480,7 +480,7 @@ smb2_verify_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) > unsigned int rc; > char server_response_sig[16]; > struct smb2_sync_hdr *shdr = > - (struct smb2_sync_hdr *)rqst->rq_iov[1].iov_base; > + (struct smb2_sync_hdr *)rqst->rq_iov[0].iov_base; cifs_readv_receive() does 1543 /* set up first iov for signature check */ 1544 rdata->iov[0].iov_base = buf; 1545 rdata->iov[0].iov_len = 4; 1546 rdata->iov[1].iov_base = buf + 4; 1547 rdata->iov[1].iov_len = server->total_read - 4; 1548 cifs_dbg(FYI, "0: iov_base=%p iov_len=%u\n", which is later used by smb2_readv_callback() to check a read rsp signature. With the change above signature checking won't work. We should probably leave this untouched and have iov[0].iov_base as NULL later when we zero preamble_header_size. Also please fix the above "+/-4" in cifs_readv_receive() to use preamble_header_size. > > if ((shdr->Command == SMB2_NEGOTIATE) || > (shdr->Command == SMB2_SESSION_SETUP) || > @@ -604,15 +604,13 @@ int > smb2_check_receive(struct mid_q_entry *mid, struct TCP_Server_Info *server, > bool log_error) > { > - unsigned int len = mid->resp_buf_size; > - struct kvec iov[2]; > + unsigned int len = server->total_read; > + struct kvec iov[1]; > struct smb_rqst rqst = { .rq_iov = iov, > - .rq_nvec = 2 }; > + .rq_nvec = 1 }; > > iov[0].iov_base = (char *)mid->resp_buf; > - iov[0].iov_len = 4; header_preamble_size can be used instead of 4 as a preparation. All places of redundant use of header_preamble_size should be clean at once in subsequent patch after zeroing it. > - iov[1].iov_base = (char *)mid->resp_buf + 4; > - iov[1].iov_len = len; > + iov[0].iov_len = mid->resp_buf_size; > > dump_smb(mid->resp_buf, min_t(u32, 80, len)); > /* convert the length into a more usable form */ > -- > 2.13.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best regards, Pavel Shilovsky -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index e8830f076a7f..ea426bb97872 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -874,7 +874,11 @@ cifs_demultiplex_thread(void *p) length = cifs_read_from_socket(server, buf, pdu_length); if (length < 0) continue; - server->total_read = length; + + if (server->vals->header_preamble_size == 0) + server->total_read = 0; + else + server->total_read = length; /* * The right amount was read from socket - 4 bytes, diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 68ea8491c160..f73bcc825002 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -131,25 +131,20 @@ static __u32 get_neg_ctxt_len(struct smb2_hdr *hdr, __u32 len, __u32 non_ctxlen, #endif /* CIFS_SMB311 */ int -smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) +smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *srvr) { - struct smb2_pdu *pdu = (struct smb2_pdu *)buf; - struct smb2_hdr *hdr = &pdu->hdr; - struct smb2_sync_hdr *shdr = get_sync_hdr(buf); + struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)buf; + struct smb2_sync_pdu *pdu = (struct smb2_sync_pdu *)shdr; __u64 mid; - __u32 len = get_rfc1002_length(buf); __u32 clc_len; /* calculated length */ int command; - - /* BB disable following printk later */ - cifs_dbg(FYI, "%s length: 0x%x, smb_buf_length: 0x%x\n", - __func__, length, len); + int pdu_size = sizeof(struct smb2_sync_pdu); + int hdr_size = sizeof(struct smb2_sync_hdr); /* * Add function to do table lookup of StructureSize by command * ie Validate the wct via smb2_struct_sizes table above */ - if (shdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM) { struct smb2_transform_hdr *thdr = (struct smb2_transform_hdr *)buf; @@ -173,8 +168,8 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) } mid = le64_to_cpu(shdr->MessageId); - if (length < sizeof(struct smb2_pdu)) { - if ((length >= sizeof(struct smb2_hdr)) + if (len < pdu_size) { + if ((len >= hdr_size) && (shdr->Status != 0)) { pdu->StructureSize2 = 0; /* @@ -227,13 +222,7 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) } } - if (srvr->vals->header_preamble_size + len != length) { - cifs_dbg(VFS, "Total length %u RFC1002 length %zu mismatch mid %llu\n", - length, srvr->vals->header_preamble_size + len, mid); - return 1; - } - - clc_len = smb2_calc_size(hdr); + clc_len = smb2_calc_size(buf); #ifdef CONFIG_CIFS_SMB311 if (shdr->Command == SMB2_NEGOTIATE) @@ -305,15 +294,14 @@ static const bool has_smb2_data_area[NUMBER_OF_SMB2_COMMANDS] = { * area and the offset to it (from the beginning of the smb are also returned. */ char * -smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *hdr) +smb2_get_data_area_len(int *off, int *len, struct smb2_sync_hdr *shdr) { - struct smb2_sync_hdr *shdr = get_sync_hdr(hdr); *off = 0; *len = 0; /* error responses do not have data area */ if (shdr->Status && shdr->Status != STATUS_MORE_PROCESSING_REQUIRED && - (((struct smb2_err_rsp *)hdr)->StructureSize) == + (((struct smb2_err_rsp *)shdr)->StructureSize) == SMB2_ERROR_STRUCTURE_SIZE2) return NULL; @@ -325,42 +313,44 @@ smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *hdr) switch (shdr->Command) { case SMB2_NEGOTIATE: *off = le16_to_cpu( - ((struct smb2_negotiate_rsp *)hdr)->SecurityBufferOffset); + ((struct smb2_sess_setup_rsp *)shdr)->SecurityBufferOffset); *len = le16_to_cpu( - ((struct smb2_negotiate_rsp *)hdr)->SecurityBufferLength); + ((struct smb2_sess_setup_rsp *)shdr)->SecurityBufferLength); break; case SMB2_SESSION_SETUP: *off = le16_to_cpu( - ((struct smb2_sess_setup_rsp *)hdr)->SecurityBufferOffset); + ((struct smb2_sess_setup_rsp *)shdr)->SecurityBufferOffset); *len = le16_to_cpu( - ((struct smb2_sess_setup_rsp *)hdr)->SecurityBufferLength); + ((struct smb2_sess_setup_rsp *)shdr)->SecurityBufferLength); break; case SMB2_CREATE: *off = le32_to_cpu( - ((struct smb2_create_rsp *)hdr)->CreateContextsOffset); + ((struct smb2_create_rsp *)shdr)->CreateContextsOffset); *len = le32_to_cpu( - ((struct smb2_create_rsp *)hdr)->CreateContextsLength); + ((struct smb2_create_rsp *)shdr)->CreateContextsLength); break; case SMB2_QUERY_INFO: *off = le16_to_cpu( - ((struct smb2_query_info_rsp *)hdr)->OutputBufferOffset); + ((struct smb2_query_info_rsp *)shdr)->OutputBufferOffset); *len = le32_to_cpu( - ((struct smb2_query_info_rsp *)hdr)->OutputBufferLength); + ((struct smb2_query_info_rsp *)shdr)->OutputBufferLength); break; case SMB2_READ: - *off = ((struct smb2_read_rsp *)hdr)->DataOffset; - *len = le32_to_cpu(((struct smb2_read_rsp *)hdr)->DataLength); + /* TODO: is this a bug ? */ + *off = ((struct smb2_read_rsp *)shdr)->DataOffset; + *len = le32_to_cpu(((struct smb2_read_rsp *)shdr)->DataLength); break; case SMB2_QUERY_DIRECTORY: *off = le16_to_cpu( - ((struct smb2_query_directory_rsp *)hdr)->OutputBufferOffset); + ((struct smb2_query_directory_rsp *)shdr)->OutputBufferOffset); *len = le32_to_cpu( - ((struct smb2_query_directory_rsp *)hdr)->OutputBufferLength); + ((struct smb2_query_directory_rsp *)shdr)->OutputBufferLength); break; case SMB2_IOCTL: *off = le32_to_cpu( - ((struct smb2_ioctl_rsp *)hdr)->OutputOffset); - *len = le32_to_cpu(((struct smb2_ioctl_rsp *)hdr)->OutputCount); + ((struct smb2_ioctl_rsp *)shdr)->OutputOffset); + *len = le32_to_cpu( + ((struct smb2_ioctl_rsp *)shdr)->OutputCount); break; case SMB2_CHANGE_NOTIFY: default: @@ -405,13 +395,12 @@ smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *hdr) unsigned int smb2_calc_size(void *buf) { - struct smb2_pdu *pdu = (struct smb2_pdu *)buf; - struct smb2_hdr *hdr = &pdu->hdr; - struct smb2_sync_hdr *shdr = get_sync_hdr(hdr); + struct smb2_sync_pdu *pdu = (struct smb2_sync_pdu *)buf; + struct smb2_sync_hdr *shdr = &pdu->sync_hdr; int offset; /* the offset from the beginning of SMB to data area */ int data_length; /* the length of the variable length data area */ /* Structure Size has already been checked to make sure it is 64 */ - int len = 4 + le16_to_cpu(shdr->StructureSize); + int len = le16_to_cpu(shdr->StructureSize); /* * StructureSize2, ie length of fixed parameter area has already @@ -422,7 +411,7 @@ smb2_calc_size(void *buf) if (has_smb2_data_area[le16_to_cpu(shdr->Command)] == false) goto calc_size_exit; - smb2_get_data_area_len(&offset, &data_length, hdr); + smb2_get_data_area_len(&offset, &data_length, shdr); cifs_dbg(FYI, "SMB2 data length %d offset %d\n", data_length, offset); if (data_length > 0) { @@ -430,15 +419,14 @@ smb2_calc_size(void *buf) * Check to make sure that data area begins after fixed area, * Note that last byte of the fixed area is part of data area * for some commands, typically those with odd StructureSize, - * so we must add one to the calculation (and 4 to account for - * the size of the RFC1001 hdr. + * so we must add one to the calculation. */ - if (offset + 4 + 1 < len) { + if (offset + 1 < len) { cifs_dbg(VFS, "data area offset %d overlaps SMB2 header %d\n", - offset + 4 + 1, len); + offset + 1, len); data_length = 0; } else { - len = 4 + offset + data_length; + len = offset + data_length; } } calc_size_exit: @@ -621,7 +609,7 @@ smb2_is_valid_lease_break(char *buffer) bool smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) { - struct smb2_oplock_break_rsp *rsp = (struct smb2_oplock_break_rsp *)buffer; + struct smb2_oplock_break *rsp = (struct smb2_oplock_break *)buffer; struct list_head *tmp, *tmp1, *tmp2; struct cifs_ses *ses; struct cifs_tcon *tcon; @@ -630,7 +618,7 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) cifs_dbg(FYI, "Checking for oplock break\n"); - if (rsp->hdr.sync_hdr.Command != SMB2_OPLOCK_BREAK) + if (rsp->sync_hdr.Command != SMB2_OPLOCK_BREAK) return false; if (rsp->StructureSize != @@ -721,8 +709,8 @@ smb2_cancelled_close_fid(struct work_struct *work) int smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server) { - struct smb2_sync_hdr *sync_hdr = get_sync_hdr(buffer); - struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)buffer; + struct smb2_sync_hdr *sync_hdr = (struct smb2_sync_hdr *)buffer; + struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)sync_hdr; struct cifs_tcon *tcon; struct close_cancelled_open *cancelled; diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index b4ae932ea134..945a7b1ed8d7 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -123,7 +123,8 @@ smb2_get_credits_field(struct TCP_Server_Info *server, const int optype) static unsigned int smb2_get_credits(struct mid_q_entry *mid) { - struct smb2_sync_hdr *shdr = get_sync_hdr(mid->resp_buf); + char *buf = mid->resp_buf; + struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)buf; return le16_to_cpu(shdr->CreditRequest); } @@ -2054,12 +2055,11 @@ smb2_dir_needs_close(struct cifsFileInfo *cfile) } static void -fill_transform_hdr(struct TCP_Server_Info *server, - struct smb2_transform_hdr *tr_hdr, struct smb_rqst *old_rq) +fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len, + struct smb_rqst *old_rq) { struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)old_rq->rq_iov[1].iov_base; - unsigned int orig_len = get_rfc1002_length(old_rq->rq_iov[0].iov_base); memset(tr_hdr, 0, sizeof(struct smb2_transform_hdr)); tr_hdr->ProtocolId = SMB2_TRANSFORM_PROTO_NUM; @@ -2067,8 +2067,6 @@ fill_transform_hdr(struct TCP_Server_Info *server, tr_hdr->Flags = cpu_to_le16(0x01); get_random_bytes(&tr_hdr->Nonce, SMB3_AES128CMM_NONCE); memcpy(&tr_hdr->SessionId, &shdr->SessionId, 8); - inc_rfc1001_len(tr_hdr, sizeof(struct smb2_transform_hdr) - server->vals->header_preamble_size); - inc_rfc1001_len(tr_hdr, orig_len); } /* We can not use the normal sg_set_buf() as we will sometimes pass a @@ -2080,11 +2078,16 @@ static inline void smb2_sg_set_buf(struct scatterlist *sg, const void *buf, sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf)); } +/* Assumes: + * rqst->rq_iov[0] is rfc1002 length + * rqst->rq_iov[1] is tranform header + * rqst->rq_iov[2+] data to be encrypted/decrypted + */ static struct scatterlist * init_sg(struct smb_rqst *rqst, u8 *sign) { - unsigned int sg_len = rqst->rq_nvec + rqst->rq_npages + 1; - unsigned int assoc_data_len = sizeof(struct smb2_transform_hdr) - 24; + unsigned int sg_len = rqst->rq_nvec + rqst->rq_npages; + unsigned int assoc_data_len = sizeof(struct smb2_transform_hdr) - 20; struct scatterlist *sg; unsigned int i; unsigned int j; @@ -2094,10 +2097,10 @@ init_sg(struct smb_rqst *rqst, u8 *sign) return NULL; sg_init_table(sg, sg_len); - smb2_sg_set_buf(&sg[0], rqst->rq_iov[0].iov_base + 24, assoc_data_len); - for (i = 1; i < rqst->rq_nvec; i++) - smb2_sg_set_buf(&sg[i], rqst->rq_iov[i].iov_base, - rqst->rq_iov[i].iov_len); + smb2_sg_set_buf(&sg[0], rqst->rq_iov[1].iov_base + 20, assoc_data_len); + for (i = 1; i < rqst->rq_nvec - 1; i++) + smb2_sg_set_buf(&sg[i], rqst->rq_iov[i+1].iov_base, + rqst->rq_iov[i+1].iov_len); for (j = 0; i < sg_len - 1; i++, j++) { unsigned int len = (j < rqst->rq_npages - 1) ? rqst->rq_pagesz : rqst->rq_tailsz; @@ -2129,9 +2132,10 @@ smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key) } /* * Encrypt or decrypt @rqst message. @rqst has the following format: - * iov[0] - transform header (associate data), - * iov[1-N] and pages - data to encrypt. - * On success return encrypted data in iov[1-N] and pages, leave iov[0] + * iov[0] - rfc1002 length + * iov[1] - transform header (associate data), + * iov[2-N] and pages - data to encrypt. + * On success return encrypted data in iov[2-N] and pages, leave iov[0-1] * untouched. */ static int @@ -2226,6 +2230,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc) return rc; } +/* + * This is called from smb_send_rqst. At this point we have the rfc1002 + * header as the first element in the vector. + */ static int smb3_init_transform_rq(struct TCP_Server_Info *server, struct smb_rqst *new_rq, struct smb_rqst *old_rq) @@ -2234,6 +2242,7 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, struct smb_rqst *new_rq, struct page **pages; struct smb2_transform_hdr *tr_hdr; unsigned int npages = old_rq->rq_npages; + unsigned int orig_len = get_rfc1002_length(old_rq->rq_iov[0].iov_base); int i; int rc = -ENOMEM; @@ -2252,24 +2261,34 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, struct smb_rqst *new_rq, goto err_free_pages; } - iov = kmalloc_array(old_rq->rq_nvec, sizeof(struct kvec), GFP_KERNEL); + /* Make space for one extra iov to hold the transform header */ + iov = kmalloc_array(old_rq->rq_nvec + 1, sizeof(struct kvec), + GFP_KERNEL); if (!iov) goto err_free_pages; /* copy all iovs from the old except the 1st one (rfc1002 length) */ - memcpy(&iov[1], &old_rq->rq_iov[1], + memcpy(&iov[2], &old_rq->rq_iov[1], sizeof(struct kvec) * (old_rq->rq_nvec - 1)); + /* copy the rfc1002 iov */ + iov[0].iov_base = old_rq->rq_iov[0].iov_base; + iov[0].iov_len = old_rq->rq_iov[0].iov_len; + new_rq->rq_iov = iov; - new_rq->rq_nvec = old_rq->rq_nvec; + new_rq->rq_nvec = old_rq->rq_nvec + 1; tr_hdr = kmalloc(sizeof(struct smb2_transform_hdr), GFP_KERNEL); if (!tr_hdr) goto err_free_iov; - /* fill the 1st iov with a transform header */ - fill_transform_hdr(server, tr_hdr, old_rq); - new_rq->rq_iov[0].iov_base = tr_hdr; - new_rq->rq_iov[0].iov_len = sizeof(struct smb2_transform_hdr); + /* fill the 2nd iov with a transform header */ + fill_transform_hdr(tr_hdr, orig_len, old_rq); + new_rq->rq_iov[1].iov_base = tr_hdr; + new_rq->rq_iov[1].iov_len = sizeof(struct smb2_transform_hdr); + + /* Update rfc1002 header */ + inc_rfc1001_len(new_rq->rq_iov[0].iov_base, + sizeof(struct smb2_transform_hdr)); /* copy pages form the old */ for (i = 0; i < npages; i++) { @@ -2309,7 +2328,7 @@ smb3_free_transform_rq(struct smb_rqst *rqst) put_page(rqst->rq_pages[i]); kfree(rqst->rq_pages); /* free transform header */ - kfree(rqst->rq_iov[0].iov_base); + kfree(rqst->rq_iov[1].iov_base); kfree(rqst->rq_iov); } @@ -2326,18 +2345,19 @@ decrypt_raw_data(struct TCP_Server_Info *server, char *buf, unsigned int buf_data_size, struct page **pages, unsigned int npages, unsigned int page_data_size) { - struct kvec iov[2]; + struct kvec iov[3]; struct smb_rqst rqst = {NULL}; - struct smb2_hdr *hdr; int rc; - iov[0].iov_base = buf; - iov[0].iov_len = sizeof(struct smb2_transform_hdr); - iov[1].iov_base = buf + sizeof(struct smb2_transform_hdr); - iov[1].iov_len = buf_data_size; + iov[0].iov_base = NULL; + iov[0].iov_len = 0; + iov[1].iov_base = buf; + iov[1].iov_len = sizeof(struct smb2_transform_hdr); + iov[2].iov_base = buf + sizeof(struct smb2_transform_hdr); + iov[2].iov_len = buf_data_size; rqst.rq_iov = iov; - rqst.rq_nvec = 2; + rqst.rq_nvec = 3; rqst.rq_pages = pages; rqst.rq_npages = npages; rqst.rq_pagesz = PAGE_SIZE; @@ -2349,10 +2369,9 @@ decrypt_raw_data(struct TCP_Server_Info *server, char *buf, if (rc) return rc; - memmove(buf + server->vals->header_preamble_size, iov[1].iov_base, buf_data_size); - hdr = (struct smb2_hdr *)buf; - hdr->smb2_buf_length = cpu_to_be32(buf_data_size + page_data_size); - server->total_read = buf_data_size + page_data_size + server->vals->header_preamble_size; + memmove(buf + server->vals->header_preamble_size, iov[2].iov_base, buf_data_size); + + server->total_read = buf_data_size + page_data_size; return rc; } @@ -3106,8 +3125,8 @@ struct smb_version_values smb20_values = { .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, - .header_size = sizeof(struct smb2_hdr), - .header_preamble_size = 4, + .header_size = sizeof(struct smb2_sync_hdr), + .header_preamble_size = 0, .max_header_size = MAX_SMB2_HDR_SIZE, .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, .lock_cmd = SMB2_LOCK, @@ -3127,8 +3146,8 @@ struct smb_version_values smb21_values = { .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, - .header_size = sizeof(struct smb2_hdr), - .header_preamble_size = 4, + .header_size = sizeof(struct smb2_sync_hdr), + .header_preamble_size = 0, .max_header_size = MAX_SMB2_HDR_SIZE, .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, .lock_cmd = SMB2_LOCK, @@ -3148,8 +3167,8 @@ struct smb_version_values smb3any_values = { .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, - .header_size = sizeof(struct smb2_hdr), - .header_preamble_size = 4, + .header_size = sizeof(struct smb2_sync_hdr), + .header_preamble_size = 0, .max_header_size = MAX_SMB2_HDR_SIZE, .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, .lock_cmd = SMB2_LOCK, @@ -3169,8 +3188,8 @@ struct smb_version_values smbdefault_values = { .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, - .header_size = sizeof(struct smb2_hdr), - .header_preamble_size = 4, + .header_size = sizeof(struct smb2_sync_hdr), + .header_preamble_size = 0, .max_header_size = MAX_SMB2_HDR_SIZE, .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, .lock_cmd = SMB2_LOCK, @@ -3190,8 +3209,8 @@ struct smb_version_values smb30_values = { .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, - .header_size = sizeof(struct smb2_hdr), - .header_preamble_size = 4, + .header_size = sizeof(struct smb2_sync_hdr), + .header_preamble_size = 0, .max_header_size = MAX_SMB2_HDR_SIZE, .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, .lock_cmd = SMB2_LOCK, @@ -3211,8 +3230,8 @@ struct smb_version_values smb302_values = { .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, - .header_size = sizeof(struct smb2_hdr), - .header_preamble_size = 4, + .header_size = sizeof(struct smb2_sync_hdr), + .header_preamble_size = 0, .max_header_size = MAX_SMB2_HDR_SIZE, .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, .lock_cmd = SMB2_LOCK, @@ -3233,8 +3252,8 @@ struct smb_version_values smb311_values = { .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, .unlock_lock_type = SMB2_LOCKFLAG_UNLOCK, - .header_size = sizeof(struct smb2_hdr), - .header_preamble_size = 4, + .header_size = sizeof(struct smb2_sync_hdr), + .header_preamble_size = 0, .max_header_size = MAX_SMB2_HDR_SIZE, .read_rsp_size = sizeof(struct smb2_read_rsp) - 1, .lock_cmd = SMB2_LOCK, diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index d7f394c41c0d..be18e653ecca 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -689,7 +689,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses) server->capabilities |= SMB2_NT_FIND | SMB2_LARGE_FILES; security_blob = smb2_get_data_area_len(&blob_offset, &blob_length, - &rsp->hdr); + get_sync_hdr(&rsp->hdr)); /* * See MS-SMB2 section 2.2.4: if no blob, client picks default which * for us will be @@ -1990,7 +1990,6 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, { struct smb2_ioctl_req *req; struct smb2_ioctl_rsp *rsp; - struct smb2_sync_hdr *shdr; struct cifs_ses *ses; struct kvec iov[2]; struct kvec rsp_iov; @@ -2111,7 +2110,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, goto ioctl_exit; } - if (get_rfc1002_length(rsp) < le32_to_cpu(rsp->OutputOffset) + *plen) { + if (rsp_iov.iov_len < le32_to_cpu(rsp->OutputOffset) + *plen) { cifs_dbg(VFS, "Malformed ioctl resp: len %d offset %d\n", *plen, le32_to_cpu(rsp->OutputOffset)); *plen = 0; @@ -2125,8 +2124,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, goto ioctl_exit; } - shdr = get_sync_hdr(rsp); - memcpy(*out_data, (char *)shdr + le32_to_cpu(rsp->OutputOffset), *plen); + memcpy(*out_data, (char *)rsp + le32_to_cpu(rsp->OutputOffset), *plen); ioctl_exit: free_rsp_buf(resp_buftype, rsp); return rc; @@ -2800,7 +2798,6 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms, int resp_buftype, rc = -EACCES; struct smb2_read_plain_req *req = NULL; struct smb2_read_rsp *rsp = NULL; - struct smb2_sync_hdr *shdr; struct kvec iov[1]; struct kvec rsp_iov; unsigned int total_len; @@ -2841,10 +2838,8 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms, *nbytes = 0; } - shdr = get_sync_hdr(rsp); - if (*buf) { - memcpy(*buf, (char *)shdr + rsp->DataOffset, *nbytes); + memcpy(*buf, (char *)rsp + rsp->DataOffset, *nbytes); free_rsp_buf(resp_buftype, rsp_iov.iov_base); } else if (resp_buftype != CIFS_NO_BUFFER) { *buf = rsp_iov.iov_base; @@ -3271,10 +3266,9 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon, cifs_buf_release(srch_inf->ntwrk_buf_start); } srch_inf->ntwrk_buf_start = (char *)rsp; - srch_inf->srch_entries_start = srch_inf->last_entry = 4 /* rfclen */ + - (char *)&rsp->hdr + le16_to_cpu(rsp->OutputBufferOffset); - /* 4 for rfc1002 length field */ - end_of_smb = get_rfc1002_length(rsp) + 4 + (char *)&rsp->hdr; + srch_inf->srch_entries_start = srch_inf->last_entry = + (char *)rsp + le16_to_cpu(rsp->OutputBufferOffset); + end_of_smb = rsp_iov.iov_len + (char *)rsp; srch_inf->entries_in_buffer = num_entries(srch_inf->srch_entries_start, end_of_smb, &srch_inf->last_entry, info_buf_size); @@ -3510,7 +3504,7 @@ SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon, __u8 oplock_level) { int rc; - struct smb2_oplock_break_req *req = NULL; + struct smb2_oplock_break *req = NULL; struct cifs_ses *ses = tcon->ses; int flags = CIFS_OBREAK_OP; unsigned int total_len; diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h index 6093e5142b2b..d8d9eb73e216 100644 --- a/fs/cifs/smb2pdu.h +++ b/fs/cifs/smb2pdu.h @@ -123,24 +123,18 @@ struct smb2_sync_pdu { } __packed; struct smb2_hdr { - __be32 smb2_buf_length; /* big endian on wire */ - /* length is only two or three bytes - with */ - /* one or two byte type preceding it that MBZ */ - struct smb2_sync_hdr sync_hdr; + struct smb2_sync_hdr sync_hdr; } __packed; struct smb2_pdu { - struct smb2_hdr hdr; - __le16 StructureSize2; /* size of wct area (varies, request specific) */ + struct smb2_hdr hdr; + __le16 StructureSize2; /* size of wct area (varies, request specific) */ } __packed; #define SMB3_AES128CMM_NONCE 11 #define SMB3_AES128GCM_NONCE 12 struct smb2_transform_hdr { - __be32 smb2_buf_length; /* big endian on wire */ - /* length is only two or three bytes - with - one or two byte type preceding it that MBZ */ __le32 ProtocolId; /* 0xFD 'S' 'M' 'B' */ __u8 Signature[16]; __u8 Nonce[16]; @@ -1154,8 +1148,7 @@ struct smb2_set_info_rsp { __le16 StructureSize; /* Must be 2 */ } __packed; -/* oplock break without an rfc1002 header */ -struct smb2_oplock_break_req { +struct smb2_oplock_break { struct smb2_sync_hdr sync_hdr; __le16 StructureSize; /* Must be 24 */ __u8 OplockLevel; @@ -1165,17 +1158,6 @@ struct smb2_oplock_break_req { __u64 VolatileFid; } __packed; -/* oplock break with an rfc1002 header */ -struct smb2_oplock_break_rsp { - struct smb2_hdr hdr; - __le16 StructureSize; /* Must be 24 */ - __u8 OplockLevel; - __u8 Reserved; - __le32 Reserved2; - __u64 PersistentFid; - __u64 VolatileFid; -} __packed; - #define SMB2_NOTIFY_BREAK_LEASE_FLAG_ACK_REQUIRED cpu_to_le32(0x01) struct smb2_lease_break { diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h index 8ba24a95db71..30cdc0aa4e8e 100644 --- a/fs/cifs/smb2proto.h +++ b/fs/cifs/smb2proto.h @@ -37,7 +37,8 @@ extern int map_smb2_to_linux_error(char *buf, bool log_err); extern int smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *server); extern unsigned int smb2_calc_size(void *buf); -extern char *smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *hdr); +extern char *smb2_get_data_area_len(int *off, int *len, + struct smb2_sync_hdr *shdr); extern __le16 *cifs_convert_path_to_utf16(const char *from, struct cifs_sb_info *cifs_sb); diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c index 8806f3f76c1d..8372a0cecdb0 100644 --- a/fs/cifs/smb2transport.c +++ b/fs/cifs/smb2transport.c @@ -480,7 +480,7 @@ smb2_verify_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) unsigned int rc; char server_response_sig[16]; struct smb2_sync_hdr *shdr = - (struct smb2_sync_hdr *)rqst->rq_iov[1].iov_base; + (struct smb2_sync_hdr *)rqst->rq_iov[0].iov_base; if ((shdr->Command == SMB2_NEGOTIATE) || (shdr->Command == SMB2_SESSION_SETUP) || @@ -604,15 +604,13 @@ int smb2_check_receive(struct mid_q_entry *mid, struct TCP_Server_Info *server, bool log_error) { - unsigned int len = mid->resp_buf_size; - struct kvec iov[2]; + unsigned int len = server->total_read; + struct kvec iov[1]; struct smb_rqst rqst = { .rq_iov = iov, - .rq_nvec = 2 }; + .rq_nvec = 1 }; iov[0].iov_base = (char *)mid->resp_buf; - iov[0].iov_len = 4; - iov[1].iov_base = (char *)mid->resp_buf + 4; - iov[1].iov_len = len; + iov[0].iov_len = mid->resp_buf_size; dump_smb(mid->resp_buf, min_t(u32, 80, len)); /* convert the length into a more usable form */
Separate out all the 4 byte rfc1002 headers so that they are no longer part of the SMB2 header structures to prepare for future work to add compounding support. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> --- fs/cifs/connect.c | 6 ++- fs/cifs/smb2misc.c | 88 ++++++++++++++++-------------------- fs/cifs/smb2ops.c | 115 ++++++++++++++++++++++++++++-------------------- fs/cifs/smb2pdu.c | 22 ++++----- fs/cifs/smb2pdu.h | 26 ++--------- fs/cifs/smb2proto.h | 3 +- fs/cifs/smb2transport.c | 12 +++-- 7 files changed, 129 insertions(+), 143 deletions(-)