>From e1d63f19517345138443055374493748dd42fcfe Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Dec 2013 11:20:49 +0100 Subject: [PATCH 1/8] s3:lib: use stack buffers in drain_socket() and default_sys_recvfile() Signed-off-by: Stefan Metzmacher --- source3/lib/recvfile.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/source3/lib/recvfile.c b/source3/lib/recvfile.c index 72f4257..f1901d4 100644 --- a/source3/lib/recvfile.c +++ b/source3/lib/recvfile.c @@ -51,7 +51,7 @@ static ssize_t default_sys_recvfile(int fromfd, size_t total = 0; size_t bufsize = MIN(TRANSFER_BUF_SIZE,count); size_t total_written = 0; - char *buffer = NULL; + char buffer[TRANSFER_BUF_SIZE]; DEBUG(10,("default_sys_recvfile: from = %d, to = %d, " "offset=%.0f, count = %lu\n", @@ -70,11 +70,6 @@ static ssize_t default_sys_recvfile(int fromfd, } } - buffer = SMB_MALLOC_ARRAY(char, bufsize); - if (buffer == NULL) { - return -1; - } - while (total < count) { size_t num_written = 0; ssize_t read_ret; @@ -84,7 +79,6 @@ static ssize_t default_sys_recvfile(int fromfd, read_ret = sys_read(fromfd, buffer, toread); if (read_ret <= 0) { /* EOF or socket error. */ - free(buffer); return -1; } @@ -119,7 +113,6 @@ static ssize_t default_sys_recvfile(int fromfd, total += read_ret; } - free(buffer); if (saved_errno) { /* Return the correct write error. */ errno = saved_errno; @@ -247,21 +240,15 @@ ssize_t drain_socket(int sockfd, size_t count) { size_t total = 0; size_t bufsize = MIN(TRANSFER_BUF_SIZE,count); - char *buffer = NULL; + char buffer[TRANSFER_BUF_SIZE]; int old_flags = 0; if (count == 0) { return 0; } - buffer = SMB_MALLOC_ARRAY(char, bufsize); - if (buffer == NULL) { - return -1; - } - old_flags = fcntl(sockfd, F_GETFL, 0); if (set_blocking(sockfd, true) == -1) { - free(buffer); return -1; } @@ -281,7 +268,6 @@ ssize_t drain_socket(int sockfd, size_t count) out: - free(buffer); if (fcntl(sockfd, F_SETFL, old_flags) == -1) { return -1; } -- 1.7.9.5 >From 8c6e3fc1b38408323bcd50f8cdf750b0637a9fee Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 4 Dec 2013 15:52:40 +0100 Subject: [PATCH 2/8] s3:smb2_write: reject writes to named pipes with recvfile This looks strange but matches the SMB1 case. We may want to clean this in a better way in both places later. Signed-off-by: Stefan Metzmacher --- source3/smbd/smb2_write.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/source3/smbd/smb2_write.c b/source3/smbd/smb2_write.c index 4e138fe..479a0ef 100644 --- a/source3/smbd/smb2_write.c +++ b/source3/smbd/smb2_write.c @@ -286,6 +286,14 @@ static struct tevent_req *smbd_smb2_write_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } + if (smbreq->unread_bytes > 0) { + /* + * Note: in_data.data is NULL in this case. + */ + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return tevent_req_post(req, ev); + } + subreq = np_write_send(state, ev, fsp->fake_file_handle, in_data.data, -- 1.7.9.5 >From 70c0bcf3fb9f10d53b5d2997708bcd2f45af4454 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 4 Dec 2013 10:41:26 +0100 Subject: [PATCH 3/8] TODO print_spool_write vs. recvfile --- source3/smbd/fileio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/smbd/fileio.c b/source3/smbd/fileio.c index b0da7a2..630e34f 100644 --- a/source3/smbd/fileio.c +++ b/source3/smbd/fileio.c @@ -317,7 +317,7 @@ ssize_t write_file(struct smb_request *req, if (fsp->print_file) { uint32_t t; int ret; - +//TODO ret = print_spool_write(fsp, data, n, pos, &t); if (ret) { errno = ret; -- 1.7.9.5 >From 4faa9589293b3554e131ba4e33b8a7f46dd8634a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 18 Nov 2013 13:45:37 +0100 Subject: [PATCH 4/8] s3:smb2_write: allow SMBD_SMB2_IN_DYN_LEN() to be 0 for the recvfile case. For recvfile we haven't read and may not allocated the dyn buffer. Signed-off-by: Stefan Metzmacher --- source3/smbd/smb2_write.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/source3/smbd/smb2_write.c b/source3/smbd/smb2_write.c index 479a0ef..8e67c7e 100644 --- a/source3/smbd/smb2_write.c +++ b/source3/smbd/smb2_write.c @@ -48,6 +48,8 @@ NTSTATUS smbd_smb2_request_process_write(struct smbd_smb2_request *req) uint64_t in_file_id_volatile; struct files_struct *in_fsp; uint32_t in_flags; + size_t in_dyn_len = 0; + uint8_t *in_dyn_ptr = NULL; struct tevent_req *subreq; status = smbd_smb2_request_verify_sizes(req, 0x31); @@ -67,7 +69,15 @@ NTSTATUS smbd_smb2_request_process_write(struct smbd_smb2_request *req) return smbd_smb2_request_error(req, NT_STATUS_INVALID_PARAMETER); } - if (in_data_length > SMBD_SMB2_IN_DYN_LEN(req)) { + if (req->smb1req != NULL && req->smb1req->unread_bytes > 0) { + in_dyn_ptr = NULL; + in_dyn_len = req->smb1req->unread_bytes; + } else { + in_dyn_ptr = SMBD_SMB2_IN_DYN_PTR(req); + in_dyn_len = SMBD_SMB2_IN_DYN_LEN(req); + } + + if (in_data_length > in_dyn_len) { return smbd_smb2_request_error(req, NT_STATUS_INVALID_PARAMETER); } @@ -79,7 +89,10 @@ NTSTATUS smbd_smb2_request_process_write(struct smbd_smb2_request *req) return smbd_smb2_request_error(req, NT_STATUS_INVALID_PARAMETER); } - in_data_buffer.data = SMBD_SMB2_IN_DYN_PTR(req); + /* + * Note: that in_dyn_ptr is NULL for the recvfile case. + */ + in_data_buffer.data = in_dyn_ptr; in_data_buffer.length = in_data_length; status = smbd_smb2_request_verify_creditcharge(req, in_data_length); @@ -348,6 +361,9 @@ static struct tevent_req *smbd_smb2_write_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } + /* + * Note: in_data.data is NULL for the recvfile case. + */ nwritten = write_file(smbreq, fsp, (const char *)in_data.data, in_offset, -- 1.7.9.5 >From 75ca11d2bc331c6a7b489390998635b8c6694aa3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 18 Nov 2013 13:46:10 +0100 Subject: [PATCH 5/8] s3:smb2_server: prepare smbd_smb2_request_verify_sizes() for the optimized recvfile() case For recvfile we haven't read and may not allocated the dyn buffer. Signed-off-by: Stefan Metzmacher --- source3/smbd/smb2_server.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 3c46efd..d659812 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -1870,6 +1870,15 @@ NTSTATUS smbd_smb2_request_verify_sizes(struct smbd_smb2_request *req, case SMB2_OP_GETINFO: min_dyn_size = 0; break; + case SMB2_OP_WRITE: + if (req->smb1req != NULL && req->smb1req->unread_bytes > 0) { + if (req->smb1req->unread_bytes < min_dyn_size) { + return NT_STATUS_INVALID_PARAMETER; + } + + min_dyn_size = 0; + } + break; } /* -- 1.7.9.5 >From 32ff49dbf947e36e4627c7140521039665270809 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 15 Nov 2013 09:12:40 +0100 Subject: [PATCH 6/8] s3:smb2_server: only allocate the required buffer in the smb2 recvfile() code path This way the buffer will likely be allocated within the existing talloc_pool, which avoids one malloc() per request. Signed-off-by: Stefan Metzmacher --- source3/smbd/globals.h | 1 + source3/smbd/smb2_server.c | 48 ++++++++++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 3baa048..cd99fe7 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -768,6 +768,7 @@ struct smbd_server_connection { struct iovec vector; bool doing_receivefile; size_t min_recv_size; + size_t pktfull; size_t pktlen; uint8_t *pktbuf; } request_read_state; diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index d659812..f67fd44 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -2883,8 +2883,7 @@ static bool is_smb2_recvfile_write(struct smbd_smb2_request_read_state *state) } DEBUG(10,("Doing recvfile write len = %u\n", - (unsigned int)(state->pktlen - - SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN))); + (unsigned int)(state->pktfull - state->pktlen))); return true; } @@ -3222,10 +3221,21 @@ again: * Read the rest of the data. */ state->doing_receivefile = false; + + state->pktbuf = talloc_realloc(state->req, + state->pktbuf, + uint8_t, + state->pktfull); + if (state->pktbuf == NULL) { + return NT_STATUS_NO_MEMORY; + } + state->vector.iov_base = (void *)(state->pktbuf + - SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN); - state->vector.iov_len = (state->pktlen - - SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN); + state->pktlen); + state->vector.iov_len = (state->pktfull - + state->pktlen); + + state->pktlen = state->pktfull; goto again; } @@ -3239,24 +3249,17 @@ again: /* * Now we analyze the NBT header */ - state->pktlen = smb2_len(state->hdr.nbt); - if (state->pktlen == 0) { + state->pktfull = smb2_len(state->hdr.nbt); + if (state->pktfull == 0) { goto got_full; } - state->pktbuf = talloc_array(state->req, uint8_t, state->pktlen); - if (state->pktbuf == NULL) { - return NT_STATUS_NO_MEMORY; - } - - state->vector.iov_base = (void *)state->pktbuf; - if (state->min_recv_size != 0) { min_recvfile_size = SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN; min_recvfile_size += state->min_recv_size; } - if (state->pktlen > min_recvfile_size) { + if (state->pktfull > min_recvfile_size) { /* * Might be a receivefile write. Read the SMB2 HEADER + * SMB2_WRITE header first. Set 'doing_receivefile' @@ -3265,12 +3268,20 @@ again: * not suitable then we'll just read the rest of the data * the next time this function is called. */ - state->vector.iov_len = SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN; + state->pktlen = SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN; state->doing_receivefile = true; } else { - state->vector.iov_len = state->pktlen; + state->pktlen = state->pktfull; } + state->pktbuf = talloc_array(state->req, uint8_t, state->pktlen); + if (state->pktbuf == NULL) { + return NT_STATUS_NO_MEMORY; + } + + state->vector.iov_base = (void *)state->pktbuf; + state->vector.iov_len = state->pktlen; + goto again; got_full: @@ -3309,8 +3320,7 @@ got_full: if (req->smb1req == NULL) { return NT_STATUS_NO_MEMORY; } - req->smb1req->unread_bytes = - state->pktlen - SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN; + req->smb1req->unread_bytes = state->pktfull - state->pktlen; } ZERO_STRUCTP(state); -- 1.7.9.5 >From 6c003f03e863503201640c8a99be8eff658aabae Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 4 Dec 2013 22:12:23 +0100 Subject: [PATCH 7/8] s3:smb2_server: make use of unlikely() Signed-off-by: Stefan Metzmacher --- source3/smbd/smb2_server.c | 194 ++++++++++++++++++++++---------------------- 1 file changed, 98 insertions(+), 96 deletions(-) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index f67fd44..45579f9 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -149,7 +149,7 @@ static const struct smbd_smb2_dispatch_table { const char *smb2_opcode_name(uint16_t opcode) { - if (opcode >= ARRAY_SIZE(smbd_smb2_table)) { + if (unlikely(opcode >= ARRAY_SIZE(smbd_smb2_table))) { return "Bad SMB2 opcode"; } return smbd_smb2_table[opcode].name; @@ -159,7 +159,7 @@ static const struct smbd_smb2_dispatch_table *smbd_smb2_call(uint16_t opcode) { const struct smbd_smb2_dispatch_table *ret = NULL; - if (opcode >= ARRAY_SIZE(smbd_smb2_table)) { + if (unlikely(opcode >= ARRAY_SIZE(smbd_smb2_table))) { return NULL; } @@ -188,11 +188,11 @@ static void print_req_vectors(const struct smbd_smb2_request *req) bool smbd_is_smb2_header(const uint8_t *inbuf, size_t size) { - if (size < (4 + SMB2_HDR_BODY)) { + if (unlikely(size < (4 + SMB2_HDR_BODY))) { return false; } - if (IVAL(inbuf, 4) != SMB2_MAGIC) { + if (unlikely(IVAL(inbuf, 4) != SMB2_MAGIC)) { return false; } @@ -211,7 +211,7 @@ static NTSTATUS smbd_initialize_smb2(struct smbd_server_connection *sconn) sconn->smb2.max_credits = lp_smb2_max_credits(); sconn->smb2.credits_bitmap = bitmap_talloc(sconn, sconn->smb2.max_credits); - if (sconn->smb2.credits_bitmap == NULL) { + if (unlikely(sconn->smb2.credits_bitmap == NULL)) { return NT_STATUS_NO_MEMORY; } @@ -221,7 +221,7 @@ static NTSTATUS smbd_initialize_smb2(struct smbd_server_connection *sconn) TEVENT_FD_READ, smbd_smb2_connection_handler, sconn); - if (sconn->smb2.fde == NULL) { + if (unlikely(sconn->smb2.fde == NULL)) { return NT_STATUS_NO_MEMORY; } @@ -270,15 +270,15 @@ static struct smbd_smb2_request *smbd_smb2_request_allocate(TALLOC_CTX *mem_ctx) #if 0 /* Enable this to find subtle valgrind errors. */ mem_pool = talloc_init("smbd_smb2_request_allocate"); -#else - mem_pool = talloc_tos(); -#endif if (mem_pool == NULL) { return NULL; } +#else + mem_pool = talloc_tos(); +#endif req = talloc_zero(mem_pool, struct smbd_smb2_request); - if (req == NULL) { + if (unlikely(req == NULL)) { talloc_free(mem_pool); return NULL; } @@ -335,19 +335,19 @@ static NTSTATUS smbd_smb2_inbuf_parse_compound(struct smbXsrv_connection *conn, tf_len = 0; } - if (len < 4) { + if (unlikely(len < 4)) { DEBUG(10, ("%d bytes left, expected at least %d\n", (int)len, 4)); goto inval; } - if (IVAL(hdr, 0) == SMB2_TF_MAGIC) { + if (unlikely(IVAL(hdr, 0) == SMB2_TF_MAGIC)) { struct smbXsrv_session *s = NULL; uint64_t uid; struct iovec tf_iov[2]; NTSTATUS status; size_t enc_len; - if (conn->protocol < PROTOCOL_SMB2_24) { + if (unlikely(conn->protocol < PROTOCOL_SMB2_24)) { DEBUG(10, ("Got SMB2_TRANSFORM header, " "but dialect[0x%04X] is used\n", conn->smb2.server.dialect)); @@ -363,7 +363,7 @@ static NTSTATUS smbd_smb2_inbuf_parse_compound(struct smbXsrv_connection *conn, goto inval; } - if (len < SMB2_TF_HDR_SIZE) { + if (unlikely(len < SMB2_TF_HDR_SIZE)) { DEBUG(1, ("%d bytes left, expected at least %d\n", (int)len, SMB2_TF_HDR_SIZE)); goto inval; @@ -376,7 +376,7 @@ static NTSTATUS smbd_smb2_inbuf_parse_compound(struct smbXsrv_connection *conn, enc_len = IVAL(tf, SMB2_TF_MSG_SIZE); uid = BVAL(tf, SMB2_TF_SESSION_ID); - if (len < SMB2_TF_HDR_SIZE + enc_len) { + if (unlikely(len < SMB2_TF_HDR_SIZE + enc_len)) { DEBUG(1, ("%d bytes left, expected at least %d\n", (int)len, (int)(SMB2_TF_HDR_SIZE + enc_len))); @@ -384,7 +384,7 @@ static NTSTATUS smbd_smb2_inbuf_parse_compound(struct smbXsrv_connection *conn, } status = smb2srv_session_lookup(conn, uid, now, &s); - if (s == NULL) { + if (unlikely(s == NULL)) { DEBUG(1, ("invalid session[%llu] in " "SMB2_TRANSFORM header\n", (unsigned long long)uid)); @@ -400,7 +400,7 @@ static NTSTATUS smbd_smb2_inbuf_parse_compound(struct smbXsrv_connection *conn, status = smb2_signing_decrypt_pdu(s->global->decryption_key, conn->protocol, tf_iov, 2); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { TALLOC_FREE(iov); return status; } @@ -413,17 +413,17 @@ static NTSTATUS smbd_smb2_inbuf_parse_compound(struct smbXsrv_connection *conn, * We need the header plus the body length field */ - if (len < SMB2_HDR_BODY + 2) { + if (unlikely(len < SMB2_HDR_BODY + 2)) { DEBUG(10, ("%d bytes left, expected at least %d\n", (int)len, SMB2_HDR_BODY)); goto inval; } - if (IVAL(hdr, 0) != SMB2_MAGIC) { + if (unlikely(IVAL(hdr, 0) != SMB2_MAGIC)) { DEBUG(10, ("Got non-SMB2 PDU: %x\n", IVAL(hdr, 0))); goto inval; } - if (SVAL(hdr, 4) != SMB2_HDR_BODY) { + if (unlikely(SVAL(hdr, 4) != SMB2_HDR_BODY)) { DEBUG(10, ("Got HDR len %d, expected %d\n", SVAL(hdr, 4), SMB2_HDR_BODY)); goto inval; @@ -434,20 +434,20 @@ static NTSTATUS smbd_smb2_inbuf_parse_compound(struct smbXsrv_connection *conn, body_size = SVAL(hdr, SMB2_HDR_BODY); if (next_command_ofs != 0) { - if (next_command_ofs < (SMB2_HDR_BODY + 2)) { + if (unlikely(next_command_ofs < (SMB2_HDR_BODY + 2))) { goto inval; } - if (next_command_ofs > full_size) { + if (unlikely(next_command_ofs > full_size)) { goto inval; } full_size = next_command_ofs; } - if (body_size < 2) { + if (unlikely(body_size < 2)) { goto inval; } body_size &= 0xfffe; - if (body_size > (full_size - SMB2_HDR_BODY)) { + if (unlikely(body_size > (full_size - SMB2_HDR_BODY))) { /* * let the caller handle the error */ @@ -469,7 +469,7 @@ static NTSTATUS smbd_smb2_inbuf_parse_compound(struct smbXsrv_connection *conn, struct iovec, num_iov + SMBD_SMB2_NUM_IOV_PER_REQ); - if (iov_tmp == NULL) { + if (unlikely(iov_tmp == NULL)) { TALLOC_FREE(iov_alloc); return NT_STATUS_NO_MEMORY; } @@ -520,7 +520,7 @@ static NTSTATUS smbd_smb2_request_create(struct smbd_server_connection *sconn, NTSTATUS status; NTTIME now; - if (size < (4 + SMB2_HDR_BODY + 2)) { + if (unlikely(size < (4 + SMB2_HDR_BODY + 2))) { DEBUG(0,("Invalid SMB2 packet length count %ld\n", (long)size)); return NT_STATUS_INVALID_PARAMETER; } @@ -528,28 +528,28 @@ static NTSTATUS smbd_smb2_request_create(struct smbd_server_connection *sconn, inhdr = inbuf + 4; protocol_version = IVAL(inhdr, SMB2_HDR_PROTOCOL_ID); - if (protocol_version != SMB2_MAGIC) { + if (unlikely(protocol_version != SMB2_MAGIC)) { DEBUG(0,("Invalid SMB packet: protocol prefix: 0x%08X\n", protocol_version)); return NT_STATUS_INVALID_PARAMETER; } cmd = SVAL(inhdr, SMB2_HDR_OPCODE); - if (cmd != SMB2_OP_NEGPROT) { + if (unlikely(cmd != SMB2_OP_NEGPROT)) { DEBUG(0,("Invalid SMB packet: first request: 0x%04X\n", cmd)); return NT_STATUS_INVALID_PARAMETER; } next_command_ofs = IVAL(inhdr, SMB2_HDR_NEXT_COMMAND); - if (next_command_ofs != 0) { + if (unlikely(next_command_ofs != 0)) { DEBUG(0,("Invalid SMB packet: next_command: 0x%08X\n", next_command_ofs)); return NT_STATUS_INVALID_PARAMETER; } req = smbd_smb2_request_allocate(sconn); - if (req == NULL) { + if (unlikely(req == NULL)) { return NT_STATUS_NO_MEMORY; } req->sconn = sconn; @@ -565,7 +565,7 @@ static NTSTATUS smbd_smb2_request_create(struct smbd_server_connection *sconn, size - NBT_HDR_SIZE, req, &req->in.vector, &req->in.vector_count); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { TALLOC_FREE(req); return status; } @@ -582,7 +582,7 @@ static bool smb2_validate_sequence_number(struct smbd_server_connection *sconn, struct bitmap *credits_bm = sconn->smb2.credits_bitmap; unsigned int offset; - if (seq_id < sconn->smb2.seqnum_low) { + if (unlikely(seq_id < sconn->smb2.seqnum_low)) { DEBUG(0,("smb2_validate_sequence_number: bad message_id " "%llu (sequence id %llu) " "(granted = %u, low = %llu, range = %u)\n", @@ -594,7 +594,7 @@ static bool smb2_validate_sequence_number(struct smbd_server_connection *sconn, return false; } - if (seq_id >= sconn->smb2.seqnum_low + sconn->smb2.seqnum_range) { + if (unlikely(seq_id >= sconn->smb2.seqnum_low + sconn->smb2.seqnum_range)) { DEBUG(0,("smb2_validate_sequence_number: bad message_id " "%llu (sequence id %llu) " "(granted = %u, low = %llu, range = %u)\n", @@ -608,7 +608,7 @@ static bool smb2_validate_sequence_number(struct smbd_server_connection *sconn, offset = seq_id % sconn->smb2.max_credits; - if (bitmap_query(credits_bm, offset)) { + if (unlikely(bitmap_query(credits_bm, offset))) { DEBUG(0,("smb2_validate_sequence_number: duplicate message_id " "%llu (sequence id %llu) " "(granted = %u, low = %llu, range = %u) " @@ -656,7 +656,7 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, uint16_t credit_charge = 1; uint64_t i; - if (opcode == SMB2_OP_CANCEL) { + if (unlikely(opcode == SMB2_OP_CANCEL)) { /* SMB2_CANCEL requests by definition resend messageids. */ return true; } @@ -675,7 +675,7 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, (unsigned long long) sconn->smb2.seqnum_low, (unsigned long long) sconn->smb2.seqnum_range)); - if (sconn->smb2.credits_granted < credit_charge) { + if (unlikely(sconn->smb2.credits_granted < credit_charge)) { DEBUG(0, ("smb2_validate_message_id: client used more " "credits than granted, mid %llu, charge %llu, " "credits_granted %llu, " @@ -706,7 +706,7 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, (unsigned long long)id)); ok = smb2_validate_sequence_number(sconn, message_id, id); - if (!ok) { + if (unlikely(!ok)) { return false; } } @@ -724,7 +724,7 @@ static NTSTATUS smbd_smb2_request_validate(struct smbd_smb2_request *req) count = req->in.vector_count; - if (count < 1 + SMBD_SMB2_NUM_IOV_PER_REQ) { + if (unlikely(count < 1 + SMBD_SMB2_NUM_IOV_PER_REQ)) { /* It's not a SMB2 request */ return NT_STATUS_INVALID_PARAMETER; } @@ -733,23 +733,25 @@ static NTSTATUS smbd_smb2_request_validate(struct smbd_smb2_request *req) struct iovec *hdr = SMBD_SMB2_IDX_HDR_IOV(req,in,idx); struct iovec *body = SMBD_SMB2_IDX_BODY_IOV(req,in,idx); const uint8_t *inhdr = NULL; + bool ok; - if (hdr->iov_len != SMB2_HDR_BODY) { + if (unlikely(hdr->iov_len != SMB2_HDR_BODY)) { return NT_STATUS_INVALID_PARAMETER; } - if (body->iov_len < 2) { + if (unlikely(body->iov_len < 2)) { return NT_STATUS_INVALID_PARAMETER; } inhdr = (const uint8_t *)hdr->iov_base; /* Check the SMB2 header */ - if (IVAL(inhdr, SMB2_HDR_PROTOCOL_ID) != SMB2_MAGIC) { + if (unlikely(IVAL(inhdr, SMB2_HDR_PROTOCOL_ID) != SMB2_MAGIC)) { return NT_STATUS_INVALID_PARAMETER; } - if (!smb2_validate_message_id(req->sconn, inhdr)) { + ok = smb2_validate_message_id(req->sconn, inhdr); + if (unlikely(!ok)) { return NT_STATUS_INVALID_PARAMETER; } } @@ -801,7 +803,7 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn, SMB_ASSERT(sconn->smb2.max_credits >= sconn->smb2.credits_granted); - if (sconn->smb2.max_credits < credit_charge) { + if (unlikely(sconn->smb2.max_credits < credit_charge)) { smbd_server_connection_terminate(sconn, "client error: credit charge > max credits\n"); return; @@ -944,7 +946,7 @@ static NTSTATUS smbd_smb2_request_setup_out(struct smbd_smb2_request *req) vector = req->out._vector; } else { vector = talloc_zero_array(req, struct iovec, count); - if (vector == NULL) { + if (unlikely(vector == NULL)) { return NT_STATUS_NO_MEMORY; } mem_ctx = vector; @@ -1229,7 +1231,7 @@ static NTSTATUS smb2_send_async_interim_response(const struct smbd_smb2_request /* Calculate outgoing credits */ smb2_calculate_credits(req, nreq); - if (DEBUGLEVEL >= 10) { + if (unlikely(DEBUGLEVEL >= 10)) { dbgtext("smb2_send_async_interim_response: nreq->current_idx = %u\n", (unsigned int)nreq->current_idx ); dbgtext("smb2_send_async_interim_response: returning %u vectors\n", @@ -1266,7 +1268,7 @@ static NTSTATUS smb2_send_async_interim_response(const struct smbd_smb2_request nreq->sconn->smb2.send_queue_len++; status = smbd_smb2_io_handler(sconn, TEVENT_FD_WRITE); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { return status; } @@ -1345,7 +1347,7 @@ NTSTATUS smbd_smb2_request_pending_queue(struct smbd_smb2_request *req, } } - if (DEBUGLEVEL >= 10) { + if (unlikely(DEBUGLEVEL >= 10)) { dbgtext("smbd_smb2_request_pending_queue: req->current_idx = %u\n", (unsigned int)req->current_idx ); print_req_vectors(req); @@ -1407,7 +1409,7 @@ NTSTATUS smbd_smb2_request_pending_queue(struct smbd_smb2_request *req, req, defer_endtime, smbd_smb2_request_pending_timer, req); - if (req->async_te == NULL) { + if (unlikely(req->async_te == NULL)) { return NT_STATUS_NO_MEMORY; } @@ -1465,7 +1467,7 @@ static void smbd_smb2_request_pending_timer(struct tevent_context *ev, */ state = talloc_zero(req->sconn, struct smbd_smb2_request_pending_state); - if (state == NULL) { + if (unlikely(state == NULL)) { smbd_server_connection_terminate(req->sconn, nt_errstr(NT_STATUS_NO_MEMORY)); return; @@ -1599,7 +1601,7 @@ static void smbd_smb2_request_pending_timer(struct tevent_context *ev, sconn->smb2.send_queue_len++; status = smbd_smb2_io_handler(sconn, TEVENT_FD_WRITE); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; @@ -1824,7 +1826,7 @@ NTSTATUS smbd_smb2_request_verify_creditcharge(struct smbd_smb2_request *req, (unsigned long long) BVAL(inhdr, SMB2_HDR_MESSAGE_ID), credit_charge, needed_charge)); - if (needed_charge > credit_charge) { + if (unlikely(needed_charge > credit_charge)) { DEBUG(2, ("CreditCharge too low, given %d, needed %d\n", credit_charge, needed_charge)); return NT_STATUS_INVALID_PARAMETER; @@ -1847,18 +1849,18 @@ NTSTATUS smbd_smb2_request_verify_sizes(struct smbd_smb2_request *req, /* * The following should be checked already. */ - if (req->in.vector_count < SMBD_SMB2_NUM_IOV_PER_REQ) { + if (unlikely(req->in.vector_count < SMBD_SMB2_NUM_IOV_PER_REQ)) { return NT_STATUS_INTERNAL_ERROR; } - if (req->current_idx > max_idx) { + if (unlikely(req->current_idx > max_idx)) { return NT_STATUS_INTERNAL_ERROR; } inhdr_v = SMBD_SMB2_IN_HDR_IOV(req); - if (inhdr_v->iov_len != SMB2_HDR_BODY) { + if (unlikely(inhdr_v->iov_len != SMB2_HDR_BODY)) { return NT_STATUS_INTERNAL_ERROR; } - if (SMBD_SMB2_IN_BODY_LEN(req) < 2) { + if (unlikely(SMBD_SMB2_IN_BODY_LEN(req) < 2)) { return NT_STATUS_INTERNAL_ERROR; } @@ -1872,7 +1874,7 @@ NTSTATUS smbd_smb2_request_verify_sizes(struct smbd_smb2_request *req, break; case SMB2_OP_WRITE: if (req->smb1req != NULL && req->smb1req->unread_bytes > 0) { - if (req->smb1req->unread_bytes < min_dyn_size) { + if (unlikely(req->smb1req->unread_bytes < min_dyn_size)) { return NT_STATUS_INVALID_PARAMETER; } @@ -1886,17 +1888,17 @@ NTSTATUS smbd_smb2_request_verify_sizes(struct smbd_smb2_request *req, * where the last byte might be in the * dynamic section.. */ - if (SMBD_SMB2_IN_BODY_LEN(req) != (expected_body_size & 0xFFFFFFFE)) { + if (unlikely(SMBD_SMB2_IN_BODY_LEN(req) != (expected_body_size & 0xFFFFFFFE))) { return NT_STATUS_INVALID_PARAMETER; } - if (SMBD_SMB2_IN_DYN_LEN(req) < min_dyn_size) { + if (unlikely(SMBD_SMB2_IN_DYN_LEN(req) < min_dyn_size)) { return NT_STATUS_INVALID_PARAMETER; } inbody = SMBD_SMB2_IN_BODY_PTR(req); body_size = SVAL(inbody, 0x00); - if (body_size != expected_body_size) { + if (unlikely(body_size != expected_body_size)) { return NT_STATUS_INVALID_PARAMETER; } @@ -1936,7 +1938,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) * once the protocol is negotiated * SMB2_OP_NEGPROT is not allowed anymore */ - if (opcode == SMB2_OP_NEGPROT) { + if (unlikely(opcode == SMB2_OP_NEGPROT)) { /* drop the connection */ return NT_STATUS_INVALID_PARAMETER; } @@ -2003,17 +2005,17 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) } call = smbd_smb2_call(opcode); - if (call == NULL) { + if (unlikely(call == NULL)) { return smbd_smb2_request_error(req, NT_STATUS_INVALID_PARAMETER); } allowed_flags = SMB2_HDR_FLAG_CHAINED | SMB2_HDR_FLAG_SIGNED | SMB2_HDR_FLAG_DFS; - if (opcode == SMB2_OP_CANCEL) { + if (unlikely(opcode == SMB2_OP_CANCEL)) { allowed_flags |= SMB2_HDR_FLAG_ASYNC; } - if ((flags & ~allowed_flags) != 0) { + if (unlikely((flags & ~allowed_flags) != 0)) { return smbd_smb2_request_error(req, NT_STATUS_INVALID_PARAMETER); } @@ -2066,7 +2068,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) if (!NT_STATUS_IS_OK(session_status)) { return smbd_smb2_request_error(req, session_status); } - } else if (opcode == SMB2_OP_CANCEL) { + } else if (unlikely(opcode == SMB2_OP_CANCEL)) { /* Cancel requests are allowed to skip the signing */ } else if (signing_required) { /* @@ -2097,7 +2099,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) * calls change_to_user() on success. */ status = smbd_smb2_request_check_tcon(req); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { return smbd_smb2_request_error(req, status); } if (req->tcon->global->encryption_required) { @@ -2119,7 +2121,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) SMB_ASSERT(call->need_tcon); - if (needed > body_size) { + if (unlikely(needed > body_size)) { return smbd_smb2_request_error(req, NT_STATUS_INVALID_PARAMETER); } @@ -2128,7 +2130,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) file_id_volatile = BVAL(body, call->fileid_ofs + 8); fsp = file_fsp_smb2(req, file_id_persistent, file_id_volatile); - if (fsp == NULL) { + if (unlikely(fsp == NULL)) { if (!call->allow_invalid_fileid) { return smbd_smb2_request_error(req, NT_STATUS_FILE_CLOSED); @@ -2415,7 +2417,7 @@ static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req) * yet. */ struct tevent_immediate *im = tevent_create_immediate(req); - if (!im) { + if (unlikely(!im)) { return NT_STATUS_NO_MEMORY; } @@ -2500,7 +2502,7 @@ static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req) req->sconn->smb2.send_queue_len++; status = smbd_smb2_io_handler(sconn, TEVENT_FD_WRITE); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { return status; } @@ -2520,20 +2522,20 @@ void smbd_smb2_request_dispatch_immediate(struct tevent_context *ctx, TALLOC_FREE(im); - if (DEBUGLEVEL >= 10) { + if (unlikely(DEBUGLEVEL >= 10)) { DEBUG(10,("smbd_smb2_request_dispatch_immediate: idx[%d] of %d vectors\n", req->current_idx, req->in.vector_count)); print_req_vectors(req); } status = smbd_smb2_request_dispatch(req); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } status = smbd_smb2_request_next_incoming(sconn); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } @@ -2556,11 +2558,11 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req, (unsigned int)(dyn ? dyn->length : 0), location)); - if (body.length < 2) { + if (unlikely(body.length < 2)) { return smbd_smb2_request_error(req, NT_STATUS_INTERNAL_ERROR); } - if ((body.length % 2) != 0) { + if (unlikely((body.length % 2) != 0)) { return smbd_smb2_request_error(req, NT_STATUS_INTERNAL_ERROR); } @@ -2623,7 +2625,7 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req, new_size = old_size + pad_size; new_dyn = talloc_zero_array(req, uint8_t, new_size); - if (new_dyn == NULL) { + if (unlikely(new_dyn == NULL)) { return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY); } @@ -2656,7 +2658,7 @@ NTSTATUS smbd_smb2_request_error_ex(struct smbd_smb2_request *req, req->current_idx, nt_errstr(status), info ? " +info" : "", location)); - if (unread_bytes) { + if (unlikely(unread_bytes)) { /* Recvfile error. Drain incoming socket. */ size_t ret; @@ -2738,7 +2740,7 @@ NTSTATUS smbd_smb2_send_oplock_break(struct smbd_server_connection *sconn, } state = talloc_zero(sconn, struct smbd_smb2_send_oplock_break_state); - if (state == NULL) { + if (unlikely(state == NULL)) { return NT_STATUS_NO_MEMORY; } state->sconn = sconn; @@ -2830,7 +2832,7 @@ NTSTATUS smbd_smb2_send_oplock_break(struct smbd_server_connection *sconn, state->sconn->smb2.send_queue_len++; status = smbd_smb2_io_handler(sconn, TEVENT_FD_WRITE); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { return status; } @@ -2924,7 +2926,7 @@ static NTSTATUS smbd_smb2_request_next_incoming(struct smbd_server_connection *s /* ask for the next request */ ZERO_STRUCTP(state); state->req = smbd_smb2_request_allocate(sconn); - if (state->req == NULL) { + if (unlikely(state->req == NULL)) { return NT_STATUS_NO_MEMORY; } state->req->sconn = sconn; @@ -2945,37 +2947,37 @@ void smbd_smb2_first_negprot(struct smbd_server_connection *sconn, (unsigned int)size)); status = smbd_initialize_smb2(sconn); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } status = smbd_smb2_request_create(sconn, inbuf, size, &req); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } status = smbd_smb2_request_validate(req); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } status = smbd_smb2_request_setup_out(req); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } status = smbd_smb2_request_dispatch(req); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } status = smbd_smb2_request_next_incoming(sconn); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } @@ -3226,7 +3228,7 @@ again: state->pktbuf, uint8_t, state->pktfull); - if (state->pktbuf == NULL) { + if (unlikely(state->pktbuf == NULL)) { return NT_STATUS_NO_MEMORY; } @@ -3250,7 +3252,7 @@ again: * Now we analyze the NBT header */ state->pktfull = smb2_len(state->hdr.nbt); - if (state->pktfull == 0) { + if (unlikely(state->pktfull == 0)) { goto got_full; } @@ -3275,7 +3277,7 @@ again: } state->pktbuf = talloc_array(state->req, uint8_t, state->pktlen); - if (state->pktbuf == NULL) { + if (unlikely(state->pktbuf == NULL)) { return NT_STATUS_NO_MEMORY; } @@ -3286,7 +3288,7 @@ again: got_full: - if (state->hdr.nbt[0] != 0x00) { + if (unlikely(state->hdr.nbt[0] != 0x00)) { DEBUG(1,("ignore NBT[0x%02X] msg\n", state->hdr.nbt[0])); @@ -3311,13 +3313,13 @@ got_full: req, &req->in.vector, &req->in.vector_count); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { return status; } if (state->doing_receivefile) { req->smb1req = talloc_zero(req, struct smb_request); - if (req->smb1req == NULL) { + if (unlikely(req->smb1req == NULL)) { return NT_STATUS_NO_MEMORY; } req->smb1req->unread_bytes = state->pktfull - state->pktlen; @@ -3331,17 +3333,17 @@ got_full: req->current_idx, req->in.vector_count)); status = smbd_smb2_request_validate(req); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { return status; } status = smbd_smb2_request_setup_out(req); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { return status; } status = smbd_smb2_request_dispatch(req); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { return status; } @@ -3361,7 +3363,7 @@ got_full: } status = smbd_smb2_request_next_incoming(sconn); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { return status; } @@ -3379,7 +3381,7 @@ static void smbd_smb2_connection_handler(struct tevent_context *ev, NTSTATUS status; status = smbd_smb2_io_handler(sconn, flags); - if (!NT_STATUS_IS_OK(status)) { + if (unlikely(!NT_STATUS_IS_OK(status))) { smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } -- 1.7.9.5 >From 682f1192f03bb84d9041e8c4d199a628e51b04c2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Dec 2013 10:25:40 +0100 Subject: [PATCH 8/8] TODO:s3:smb2_server: read/write avoid tevent_req_post() --- source3/smbd/smb2_read.c | 4 ++-- source3/smbd/smb2_write.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source3/smbd/smb2_read.c b/source3/smbd/smb2_read.c index 05563b2..7413b00 100644 --- a/source3/smbd/smb2_read.c +++ b/source3/smbd/smb2_read.c @@ -483,7 +483,7 @@ static struct tevent_req *smbd_smb2_read_send(TALLOC_CTX *mem_ctx, status = schedule_smb2_sendfile_read(smb2req, state); if (NT_STATUS_IS_OK(status)) { tevent_req_done(req); - return tevent_req_post(req, ev); + return req;//tevent_req_post(req, ev); } else { if (!NT_STATUS_EQUAL(status, NT_STATUS_RETRY)) { SMB_VFS_STRICT_UNLOCK(conn, fsp, &lock); @@ -523,7 +523,7 @@ static struct tevent_req *smbd_smb2_read_send(TALLOC_CTX *mem_ctx, /* Success. */ tevent_req_done(req); } - return tevent_req_post(req, ev); + return req;//tevent_req_post(req, ev); } static void smbd_smb2_read_pipe_done(struct tevent_req *subreq) diff --git a/source3/smbd/smb2_write.c b/source3/smbd/smb2_write.c index 8e67c7e..69b713e 100644 --- a/source3/smbd/smb2_write.c +++ b/source3/smbd/smb2_write.c @@ -387,7 +387,7 @@ static struct tevent_req *smbd_smb2_write_send(TALLOC_CTX *mem_ctx, tevent_req_done(req); } - return tevent_req_post(req, ev); + return req;//tevent_req_post(req, ev); } static void smbd_smb2_write_pipe_done(struct tevent_req *subreq) -- 1.7.9.5