| Message ID | 20180807035634.30204-2-lsahlber@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
2018-08-06 20:56 GMT-07:00 Ronnie Sahlberg <lsahlber@redhat.com>: > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/cifsglob.h | 5 +++- > fs/cifs/connect.c | 77 +++++++++++++++++++++++++++++++---------------------- > fs/cifs/smb2ops.c | 58 +++++++++++++++++++++++++++++++++------- > fs/cifs/transport.c | 2 -- > 4 files changed, 98 insertions(+), 44 deletions(-) > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index 0553929e8339..2b89cf693243 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -76,6 +76,9 @@ > #define SMB_ECHO_INTERVAL_MAX 600 > #define SMB_ECHO_INTERVAL_DEFAULT 60 > > +/* maximum number of PDUs in one compound */ > +#define MAX_COMPOUND 5 > + > /* > * Default number of credits to keep available for SMB3. > * This value is chosen somewhat arbitrarily. The Windows client > @@ -458,7 +461,7 @@ struct smb_version_operations { > struct smb_rqst *, struct smb_rqst *); > int (*is_transform_hdr)(void *buf); > int (*receive_transform)(struct TCP_Server_Info *, > - struct mid_q_entry **); > + struct mid_q_entry **, int *); > enum securityEnum (*select_sectype)(struct TCP_Server_Info *, > enum securityEnum); > int (*next_header)(char *); > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index d9bd10d295a9..1d983add05a7 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -850,13 +850,13 @@ cifs_handle_standard(struct TCP_Server_Info *server, struct mid_q_entry *mid) > static int > cifs_demultiplex_thread(void *p) > { > - int length; > + int i, num_mids, length; > struct TCP_Server_Info *server = p; > unsigned int pdu_length; > unsigned int next_offset; > char *buf = NULL; > struct task_struct *task_to_wake = NULL; > - struct mid_q_entry *mid_entry; > + struct mid_q_entry *mids[MAX_COMPOUND]; > > current->flags |= PF_MEMALLOC; > cifs_dbg(FYI, "Demultiplex PID: %d\n", task_pid_nr(current)); > @@ -923,24 +923,28 @@ cifs_demultiplex_thread(void *p) > server->pdu_size = next_offset; > } > > - mid_entry = NULL; > + memset(mids, 0, sizeof(mids)); > + num_mids = 1; Isn't it cleaner to assume 0 mids here? > + > if (server->ops->is_transform_hdr && > server->ops->receive_transform && > server->ops->is_transform_hdr(buf)) { > length = server->ops->receive_transform(server, > - &mid_entry); > + mids, > + &num_mids); > } else { > - mid_entry = server->ops->find_mid(server, buf); > + mids[0] = server->ops->find_mid(server, buf); > > - if (!mid_entry || !mid_entry->receive) > - length = standard_receive3(server, mid_entry); > + if (!mids[0] || !mids[0]->receive) > + length = standard_receive3(server, mids[0]); > else > - length = mid_entry->receive(server, mid_entry); > + length = mids[0]->receive(server, mids[0]); and we can set num_mid to 1 here if mids[0] != NULL. > } > > if (length < 0) { > - if (mid_entry) > - cifs_mid_q_entry_release(mid_entry); > + for (i = 0; i < num_mids; i++) > + if (mids[i]) > + cifs_mid_q_entry_release(mids[i]); > continue; > } > > @@ -948,33 +952,42 @@ cifs_demultiplex_thread(void *p) > buf = server->bigbuf; > > server->lstrp = jiffies; > - if (mid_entry != NULL) { > - mid_entry->resp_buf_size = server->pdu_size; > - if ((mid_entry->mid_flags & MID_WAIT_CANCELLED) && > - mid_entry->mid_state == MID_RESPONSE_RECEIVED && > + if (buf && server->ops->is_oplock_break && > + server->ops->is_oplock_break(buf, server)) { We should execute this only if no mid was found for a particular response. Also we need a valid buf pointer here pointing to a decrypted response which is not the case for compounding chain. > + cifs_dbg(FYI, "Received oplock break\n"); > + goto finished_mids; > + } > + > + for (i = 0; i < num_mids; i++) { > + if (mids[i] == NULL) { > + cifs_dbg(VFS, "No task to wake, unknown frame " > + "received! NumMids %d\n", > + atomic_read(&midCount)); > + cifs_dump_mem("Received Data is: ", buf, Here is the same situation as for oplock breaks - a valid buf pointer is needed. > + HEADER_SIZE(server)); > +#ifdef CONFIG_CIFS_DEBUG2 > + if (server->ops->dump_detail) > + server->ops->dump_detail(buf, server); > + cifs_dump_mids(server); > +#endif /* CIFS_DEBUG2 */ > + break; > + } > + mids[i]->resp_buf_size = server->pdu_size; > + if ((mids[i]->mid_flags & MID_WAIT_CANCELLED) && > + mids[i]->mid_state == MID_RESPONSE_RECEIVED && > server->ops->handle_cancelled_mid) > server->ops->handle_cancelled_mid( > - mid_entry->resp_buf, > + mids[i]->resp_buf, > server); > > - if (!mid_entry->multiRsp || mid_entry->multiEnd) > - mid_entry->callback(mid_entry); > - > - cifs_mid_q_entry_release(mid_entry); > - } else if (server->ops->is_oplock_break && > - server->ops->is_oplock_break(buf, server)) { > - cifs_dbg(FYI, "Received oplock break\n"); > - } else { > - cifs_dbg(VFS, "No task to wake, unknown frame received! NumMids %d\n", > - atomic_read(&midCount)); > - cifs_dump_mem("Received Data is: ", buf, > - HEADER_SIZE(server)); > -#ifdef CONFIG_CIFS_DEBUG2 > - if (server->ops->dump_detail) > - server->ops->dump_detail(buf, server); > - cifs_dump_mids(server); > -#endif /* CIFS_DEBUG2 */ > + if (!mids[i]->multiRsp || mids[i]->multiEnd) > + mids[i]->callback(mids[i]); > } > + > + finished_mids: > + for (i = 0; i < num_mids; i++) > + if (mids[i]) > + cifs_mid_q_entry_release(mids[i]); > if (pdu_length > server->pdu_size) { > if (!allocate_buffers(server)) > continue; > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index ebc13ebebddf..0d797eb7d055 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -2942,13 +2942,18 @@ receive_encrypted_read(struct TCP_Server_Info *server, struct mid_q_entry **mid) > > static int > receive_encrypted_standard(struct TCP_Server_Info *server, > - struct mid_q_entry **mid) > + struct mid_q_entry **mid, int *num_mids) > { > - int length; > + int ret, length; > char *buf = server->smallbuf; > + struct smb2_sync_hdr *shdr; > unsigned int pdu_length = server->pdu_size; > unsigned int buf_size; > struct mid_q_entry *mid_entry; > + int next_is_large; > + char *next_buffer = NULL; > + > + *num_mids = 0; > > /* switch to large buffer if too big for a small one */ > if (pdu_length > MAX_CIFS_SMALL_BUFFER_SIZE) { > @@ -2969,6 +2974,21 @@ receive_encrypted_standard(struct TCP_Server_Info *server, > if (length) > return length; > > + next_is_large = server->large_buf; > + one_more: > + shdr = (struct smb2_sync_hdr *)buf; > + if (shdr->NextCommand) { > + if (next_is_large) { > + next_buffer = (char *)cifs_buf_get(); > + memcpy(next_buffer, server->bigbuf + shdr->NextCommand, > + pdu_length - shdr->NextCommand); > + } else { > + next_buffer = (char *)cifs_small_buf_get(); > + memcpy(next_buffer, server->smallbuf + shdr->NextCommand, > + pdu_length - shdr->NextCommand); > + } > + } > + > mid_entry = smb2_find_mid(server, buf); > if (mid_entry == NULL) > cifs_dbg(FYI, "mid not found\n"); > @@ -2976,17 +2996,36 @@ receive_encrypted_standard(struct TCP_Server_Info *server, > cifs_dbg(FYI, "mid found\n"); > mid_entry->decrypted = true; > } > + mid_entry->resp_buf_size = server->pdu_size; > > - *mid = mid_entry; > - > + mid[(*num_mids)++] = mid_entry; > + if (*num_mids >= MAX_COMPOUND) { > + cifs_dbg(VFS, "too many PDUs in compound\n"); > + return -1; > + } > if (mid_entry && mid_entry->handle) > - return mid_entry->handle(server, mid_entry); > + ret = mid_entry->handle(server, mid_entry); > + else > + ret = cifs_handle_standard(server, mid_entry); Let's suppose we didn't find a mid for a message in the middle of the chain. cifs_handle_standard() wan't called and the buffer was't saved anywhere... > + > + if (ret == 0 && shdr->NextCommand) { > + pdu_length -= shdr->NextCommand; > + server->large_buf = next_is_large; > + if (next_is_large) { > + server->bigbuf = next_buffer; > + } else { > + server->smallbuf = next_buffer; > + } ...and them we overwrite the server's buffers with the next one in the chain - leaking a current buffer. > + buf += shdr->NextCommand; > + goto one_more; > + } > > - return cifs_handle_standard(server, mid_entry); > + return ret; > } > > static int > -smb3_receive_transform(struct TCP_Server_Info *server, struct mid_q_entry **mid) > +smb3_receive_transform(struct TCP_Server_Info *server, > + struct mid_q_entry **mids, int *num_mids) > { > char *buf = server->smallbuf; > unsigned int pdu_length = server->pdu_size; > @@ -3009,10 +3048,11 @@ smb3_receive_transform(struct TCP_Server_Info *server, struct mid_q_entry **mid) > return -ECONNABORTED; > } > > + /* TODO: add support for compounds containing READ. */ > if (pdu_length > CIFSMaxBufSize + MAX_HEADER_SIZE(server)) > - return receive_encrypted_read(server, mid); > + return receive_encrypted_read(server, &mids[0]); > > - return receive_encrypted_standard(server, mid); > + return receive_encrypted_standard(server, mids, num_mids); > } > > int > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c > index 92de5c528161..3122c30eaf5c 100644 > --- a/fs/cifs/transport.c > +++ b/fs/cifs/transport.c > @@ -378,8 +378,6 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst, > return rc; > } > > -#define MAX_COMPOUND 5 > - > static int > smb_send_rqst(struct TCP_Server_Info *server, int num_rqst, > struct smb_rqst *rqst, int flags) > -- > 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 One way to overcome the problems above is to return an array of pairs {mid; buf} from receive_transform() function and then process them one by one checking for mid existence, verifying oplock breaks and dumping responses without corresponding mids (exactly the same way we do it for mid_entry and buf variables in demultiplex thread today). -- 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/cifsglob.h b/fs/cifs/cifsglob.h index 0553929e8339..2b89cf693243 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -76,6 +76,9 @@ #define SMB_ECHO_INTERVAL_MAX 600 #define SMB_ECHO_INTERVAL_DEFAULT 60 +/* maximum number of PDUs in one compound */ +#define MAX_COMPOUND 5 + /* * Default number of credits to keep available for SMB3. * This value is chosen somewhat arbitrarily. The Windows client @@ -458,7 +461,7 @@ struct smb_version_operations { struct smb_rqst *, struct smb_rqst *); int (*is_transform_hdr)(void *buf); int (*receive_transform)(struct TCP_Server_Info *, - struct mid_q_entry **); + struct mid_q_entry **, int *); enum securityEnum (*select_sectype)(struct TCP_Server_Info *, enum securityEnum); int (*next_header)(char *); diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index d9bd10d295a9..1d983add05a7 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -850,13 +850,13 @@ cifs_handle_standard(struct TCP_Server_Info *server, struct mid_q_entry *mid) static int cifs_demultiplex_thread(void *p) { - int length; + int i, num_mids, length; struct TCP_Server_Info *server = p; unsigned int pdu_length; unsigned int next_offset; char *buf = NULL; struct task_struct *task_to_wake = NULL; - struct mid_q_entry *mid_entry; + struct mid_q_entry *mids[MAX_COMPOUND]; current->flags |= PF_MEMALLOC; cifs_dbg(FYI, "Demultiplex PID: %d\n", task_pid_nr(current)); @@ -923,24 +923,28 @@ cifs_demultiplex_thread(void *p) server->pdu_size = next_offset; } - mid_entry = NULL; + memset(mids, 0, sizeof(mids)); + num_mids = 1; + if (server->ops->is_transform_hdr && server->ops->receive_transform && server->ops->is_transform_hdr(buf)) { length = server->ops->receive_transform(server, - &mid_entry); + mids, + &num_mids); } else { - mid_entry = server->ops->find_mid(server, buf); + mids[0] = server->ops->find_mid(server, buf); - if (!mid_entry || !mid_entry->receive) - length = standard_receive3(server, mid_entry); + if (!mids[0] || !mids[0]->receive) + length = standard_receive3(server, mids[0]); else - length = mid_entry->receive(server, mid_entry); + length = mids[0]->receive(server, mids[0]); } if (length < 0) { - if (mid_entry) - cifs_mid_q_entry_release(mid_entry); + for (i = 0; i < num_mids; i++) + if (mids[i]) + cifs_mid_q_entry_release(mids[i]); continue; } @@ -948,33 +952,42 @@ cifs_demultiplex_thread(void *p) buf = server->bigbuf; server->lstrp = jiffies; - if (mid_entry != NULL) { - mid_entry->resp_buf_size = server->pdu_size; - if ((mid_entry->mid_flags & MID_WAIT_CANCELLED) && - mid_entry->mid_state == MID_RESPONSE_RECEIVED && + if (buf && server->ops->is_oplock_break && + server->ops->is_oplock_break(buf, server)) { + cifs_dbg(FYI, "Received oplock break\n"); + goto finished_mids; + } + + for (i = 0; i < num_mids; i++) { + if (mids[i] == NULL) { + cifs_dbg(VFS, "No task to wake, unknown frame " + "received! NumMids %d\n", + atomic_read(&midCount)); + cifs_dump_mem("Received Data is: ", buf, + HEADER_SIZE(server)); +#ifdef CONFIG_CIFS_DEBUG2 + if (server->ops->dump_detail) + server->ops->dump_detail(buf, server); + cifs_dump_mids(server); +#endif /* CIFS_DEBUG2 */ + break; + } + mids[i]->resp_buf_size = server->pdu_size; + if ((mids[i]->mid_flags & MID_WAIT_CANCELLED) && + mids[i]->mid_state == MID_RESPONSE_RECEIVED && server->ops->handle_cancelled_mid) server->ops->handle_cancelled_mid( - mid_entry->resp_buf, + mids[i]->resp_buf, server); - if (!mid_entry->multiRsp || mid_entry->multiEnd) - mid_entry->callback(mid_entry); - - cifs_mid_q_entry_release(mid_entry); - } else if (server->ops->is_oplock_break && - server->ops->is_oplock_break(buf, server)) { - cifs_dbg(FYI, "Received oplock break\n"); - } else { - cifs_dbg(VFS, "No task to wake, unknown frame received! NumMids %d\n", - atomic_read(&midCount)); - cifs_dump_mem("Received Data is: ", buf, - HEADER_SIZE(server)); -#ifdef CONFIG_CIFS_DEBUG2 - if (server->ops->dump_detail) - server->ops->dump_detail(buf, server); - cifs_dump_mids(server); -#endif /* CIFS_DEBUG2 */ + if (!mids[i]->multiRsp || mids[i]->multiEnd) + mids[i]->callback(mids[i]); } + + finished_mids: + for (i = 0; i < num_mids; i++) + if (mids[i]) + cifs_mid_q_entry_release(mids[i]); if (pdu_length > server->pdu_size) { if (!allocate_buffers(server)) continue; diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index ebc13ebebddf..0d797eb7d055 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -2942,13 +2942,18 @@ receive_encrypted_read(struct TCP_Server_Info *server, struct mid_q_entry **mid) static int receive_encrypted_standard(struct TCP_Server_Info *server, - struct mid_q_entry **mid) + struct mid_q_entry **mid, int *num_mids) { - int length; + int ret, length; char *buf = server->smallbuf; + struct smb2_sync_hdr *shdr; unsigned int pdu_length = server->pdu_size; unsigned int buf_size; struct mid_q_entry *mid_entry; + int next_is_large; + char *next_buffer = NULL; + + *num_mids = 0; /* switch to large buffer if too big for a small one */ if (pdu_length > MAX_CIFS_SMALL_BUFFER_SIZE) { @@ -2969,6 +2974,21 @@ receive_encrypted_standard(struct TCP_Server_Info *server, if (length) return length; + next_is_large = server->large_buf; + one_more: + shdr = (struct smb2_sync_hdr *)buf; + if (shdr->NextCommand) { + if (next_is_large) { + next_buffer = (char *)cifs_buf_get(); + memcpy(next_buffer, server->bigbuf + shdr->NextCommand, + pdu_length - shdr->NextCommand); + } else { + next_buffer = (char *)cifs_small_buf_get(); + memcpy(next_buffer, server->smallbuf + shdr->NextCommand, + pdu_length - shdr->NextCommand); + } + } + mid_entry = smb2_find_mid(server, buf); if (mid_entry == NULL) cifs_dbg(FYI, "mid not found\n"); @@ -2976,17 +2996,36 @@ receive_encrypted_standard(struct TCP_Server_Info *server, cifs_dbg(FYI, "mid found\n"); mid_entry->decrypted = true; } + mid_entry->resp_buf_size = server->pdu_size; - *mid = mid_entry; - + mid[(*num_mids)++] = mid_entry; + if (*num_mids >= MAX_COMPOUND) { + cifs_dbg(VFS, "too many PDUs in compound\n"); + return -1; + } if (mid_entry && mid_entry->handle) - return mid_entry->handle(server, mid_entry); + ret = mid_entry->handle(server, mid_entry); + else + ret = cifs_handle_standard(server, mid_entry); + + if (ret == 0 && shdr->NextCommand) { + pdu_length -= shdr->NextCommand; + server->large_buf = next_is_large; + if (next_is_large) { + server->bigbuf = next_buffer; + } else { + server->smallbuf = next_buffer; + } + buf += shdr->NextCommand; + goto one_more; + } - return cifs_handle_standard(server, mid_entry); + return ret; } static int -smb3_receive_transform(struct TCP_Server_Info *server, struct mid_q_entry **mid) +smb3_receive_transform(struct TCP_Server_Info *server, + struct mid_q_entry **mids, int *num_mids) { char *buf = server->smallbuf; unsigned int pdu_length = server->pdu_size; @@ -3009,10 +3048,11 @@ smb3_receive_transform(struct TCP_Server_Info *server, struct mid_q_entry **mid) return -ECONNABORTED; } + /* TODO: add support for compounds containing READ. */ if (pdu_length > CIFSMaxBufSize + MAX_HEADER_SIZE(server)) - return receive_encrypted_read(server, mid); + return receive_encrypted_read(server, &mids[0]); - return receive_encrypted_standard(server, mid); + return receive_encrypted_standard(server, mids, num_mids); } int diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index 92de5c528161..3122c30eaf5c 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -378,8 +378,6 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst, return rc; } -#define MAX_COMPOUND 5 - static int smb_send_rqst(struct TCP_Server_Info *server, int num_rqst, struct smb_rqst *rqst, int flags)
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> --- fs/cifs/cifsglob.h | 5 +++- fs/cifs/connect.c | 77 +++++++++++++++++++++++++++++++---------------------- fs/cifs/smb2ops.c | 58 +++++++++++++++++++++++++++++++++------- fs/cifs/transport.c | 2 -- 4 files changed, 98 insertions(+), 44 deletions(-)