>From 5e83b58147fa34195874c9f73d3ae9642b3bfbcd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Dec 2013 11:20:49 +0100 Subject: [PATCH 1/9] 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..500a7e4 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[bufsize]; 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[bufsize]; 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 49a521397dde9072031396a6728b46aca01e1349 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 11 Apr 2014 00:29:48 +0200 Subject: [PATCH 2/9] s3:smbXsrv_open: allow now==0 to skip the idle_time update. Signed-off-by: Stefan Metzmacher --- source3/smbd/smbXsrv_open.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c index b15be28..c3ff3bb 100644 --- a/source3/smbd/smbXsrv_open.c +++ b/source3/smbd/smbXsrv_open.c @@ -444,7 +444,9 @@ static NTSTATUS smbXsrv_open_local_lookup(struct smbXsrv_open_table *table, return NT_STATUS_FILE_CLOSED; } - state.op->idle_time = now; + if (now != 0) { + state.op->idle_time = now; + } *_open = state.op; return state.op->status; -- 1.7.9.5 >From f5efc2d5dc0c2056b678b81370377f13a0789fc6 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 11 Apr 2014 00:51:32 +0200 Subject: [PATCH 3/9] s3:smbd: use smb1srv_open_lookup() in is_valid_writeX_buffer() It's more logical to check the fnum instead of tid here. This will make it easier to reuse the logic for SMB2 and allows per fsp recvfile detection. Signed-off-by: Stefan Metzmacher --- source3/smbd/reply.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index e58735e..da59ca7 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -4598,12 +4598,12 @@ bool is_valid_writeX_buffer(struct smbd_server_connection *sconn, const uint8_t *inbuf) { size_t numtowrite; - connection_struct *conn = NULL; unsigned int doff = 0; size_t len = smb_len_large(inbuf); - struct smbXsrv_tcon *tcon; + uint16_t fnum; + struct smbXsrv_open *op = NULL; + struct files_struct *fsp = NULL; NTSTATUS status; - NTTIME now = 0; if (is_encrypted_packet(sconn, inbuf)) { /* Can't do this on encrypted @@ -4622,19 +4622,30 @@ bool is_valid_writeX_buffer(struct smbd_server_connection *sconn, return false; } - status = smb1srv_tcon_lookup(sconn->conn, SVAL(inbuf, smb_tid), - now, &tcon); + fnum = SVAL(inbuf, smb_vwv2); + status = smb1srv_open_lookup(sconn->conn, + fnum, + 0, /* now */ + &op); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10,("is_valid_writeX_buffer: bad tid\n")); + DEBUG(10,("is_valid_writeX_buffer: bad fnum\n")); + return false; + } + fsp = op->compat; + if (fsp == NULL) { + DEBUG(10,("is_valid_writeX_buffer: bad fsp\n")); + return false; + } + if (fsp->conn == NULL) { + DEBUG(10,("is_valid_writeX_buffer: bad fsp->conn\n")); return false; } - conn = tcon->compat; - if (IS_IPC(conn)) { + if (IS_IPC(fsp->conn)) { DEBUG(10,("is_valid_writeX_buffer: IPC$ tid\n")); return false; } - if (IS_PRINT(conn)) { + if (IS_PRINT(fsp->conn)) { DEBUG(10,("is_valid_writeX_buffer: printing tid\n")); return false; } -- 1.7.9.5 >From ca3d55e92a6e1417da9959f5a7ff7da9feea854d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 11 Apr 2014 00:43:46 +0200 Subject: [PATCH 4/9] s3:smb2_server: use the same logic to avoid recvfile() for IPC/PRINT shares Signed-off-by: Stefan Metzmacher --- source3/smbd/smb2_server.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 3c46efd..13626f3 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -2841,7 +2841,19 @@ static size_t get_min_receive_file_size(struct smbd_smb2_request *smb2_req) static bool is_smb2_recvfile_write(struct smbd_smb2_request_read_state *state) { + NTSTATUS status; uint32_t flags; + uint64_t file_id_persistent; + uint64_t file_id_volatile; + struct smbXsrv_open *op = NULL; + struct files_struct *fsp = NULL; + const uint8_t *body = NULL; + + /* + * This is only called with a pktbuf + * of at least SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN + * bytes + */ if (IVAL(state->pktbuf, 0) == SMB2_TF_MAGIC) { /* Transform header. Cannot recvfile. */ @@ -2873,6 +2885,35 @@ static bool is_smb2_recvfile_write(struct smbd_smb2_request_read_state *state) return false; } + body = &state->pktbuf[SMB2_HDR_BODY]; + + file_id_persistent = BVAL(body, 0x10); + file_id_volatile = BVAL(body, 0x18); + + status = smb2srv_open_lookup(state->req->sconn->conn, + file_id_persistent, + file_id_volatile, + 0, /* now */ + &op); + if (!NT_STATUS_IS_OK(status)) { + return false; + } + + fsp = op->compat; + if (fsp == NULL) { + return false; + } + if (fsp->conn == NULL) { + return false; + } + + if (IS_IPC(fsp->conn)) { + return false; + } + if (IS_PRINT(fsp->conn)) { + return false; + } + DEBUG(10,("Doing recvfile write len = %u\n", (unsigned int)(state->pktlen - SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN))); -- 1.7.9.5 >From ebe8349172d5612d2386a69b3cb8aab4e574a769 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 11 Apr 2014 01:05:21 +0200 Subject: [PATCH 5/9] s3:smb2_server: make sure we don't try recvfile for special NBT messages Signed-off-by: Stefan Metzmacher --- source3/smbd/smb2_server.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 13626f3..4949fd2 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -3271,6 +3271,9 @@ again: /* * Now we analyze the NBT header */ + if (state->hdr.nbt[0] != 0x00) { + state->min_recv_size = 0; + } state->pktlen = smb2_len(state->hdr.nbt); if (state->pktlen == 0) { goto got_full; -- 1.7.9.5 >From 28628e8a28e447d3fd951e6e1b04cee4951f98ea Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 18 Nov 2013 13:45:37 +0100 Subject: [PATCH 6/9] 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 4e138fe..c61254f 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); @@ -340,6 +353,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 a2c8d21f87f302cf0cd6453d8e80360ca334cae3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 18 Nov 2013 13:46:10 +0100 Subject: [PATCH 7/9] 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 4949fd2..0e42b7f 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 91ad439410cab3ac7835ba319d1d48fb55448b0d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 15 Nov 2013 09:12:40 +0100 Subject: [PATCH 8/9] 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 0e42b7f..4f69679 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -2924,8 +2924,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; } @@ -3263,10 +3262,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; } @@ -3283,24 +3293,17 @@ again: if (state->hdr.nbt[0] != 0x00) { state->min_recv_size = 0; } - 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' @@ -3309,12 +3312,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: @@ -3353,8 +3364,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 9ee58034fc040591b3767be7a571f2fa3db02a54 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 11 Apr 2014 01:37:42 +0200 Subject: [PATCH 9/9] s3:smb2_server: remove unused get_min_receive_file_size() wrapper function smb2req always comes from talloc_zero(). Signed-off-by: Stefan Metzmacher --- source3/smbd/smb2_server.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 4f69679..4a7abcb 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -2837,17 +2837,6 @@ NTSTATUS smbd_smb2_send_oplock_break(struct smbd_server_connection *sconn, return NT_STATUS_OK; } -static size_t get_min_receive_file_size(struct smbd_smb2_request *smb2_req) -{ - if (smb2_req->do_signing) { - return 0; - } - if (smb2_req->do_encryption) { - return 0; - } - return (size_t)lp_min_receive_file_size(); -} - static bool is_smb2_recvfile_write(struct smbd_smb2_request_read_state *state) { NTSTATUS status; @@ -2969,7 +2958,7 @@ static NTSTATUS smbd_smb2_request_next_incoming(struct smbd_server_connection *s return NT_STATUS_NO_MEMORY; } state->req->sconn = sconn; - state->min_recv_size = get_min_receive_file_size(state->req); + state->min_recv_size = lp_min_receive_file_size(); TEVENT_FD_READABLE(sconn->smb2.fde); @@ -3337,7 +3326,7 @@ got_full: req = state->req; ZERO_STRUCTP(state); state->req = req; - state->min_recv_size = get_min_receive_file_size(state->req); + state->min_recv_size = lp_min_receive_file_size(); req = NULL; goto again; } -- 1.7.9.5