From de0c483c14eb51c2deb59587618a6e1bb6feb073 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 27 Nov 2015 19:10:01 +0100 Subject: [PATCH 1/4] libcli/smb: correctly handle STATUS_BUFFER_OVERFLOW in smb2cli_read* BUG: https://bugzilla.samba.org/show_bug.cgi?id=11623 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit b47bfce6781ea3be2b85cbef348107eda4f98860) --- libcli/smb/smb2cli_read.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/libcli/smb/smb2cli_read.c b/libcli/smb/smb2cli_read.c index 4a31622..8110b65 100644 --- a/libcli/smb/smb2cli_read.c +++ b/libcli/smb/smb2cli_read.c @@ -29,6 +29,7 @@ struct smb2cli_read_state { struct iovec *recv_iov; uint8_t *data; uint32_t data_length; + bool out_valid; }; static void smb2cli_read_done(struct tevent_req *subreq); @@ -105,8 +106,12 @@ static void smb2cli_read_done(struct tevent_req *subreq) status = smb2cli_req_recv(subreq, state, &iov, expected, ARRAY_SIZE(expected)); TALLOC_FREE(subreq); - if (tevent_req_nterror(req, status)) { - return; + if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) { + /* no error */ + } else { + if (tevent_req_nterror(req, status)) { + return; + } } data_offset = CVAL(iov[1].iov_base, 2); @@ -120,6 +125,13 @@ static void smb2cli_read_done(struct tevent_req *subreq) state->recv_iov = iov; state->data = (uint8_t *)iov[2].iov_base; + + state->out_valid = true; + + if (tevent_req_nterror(req, status)) { + return; + } + tevent_req_done(req); } @@ -129,15 +141,19 @@ NTSTATUS smb2cli_read_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, struct smb2cli_read_state *state = tevent_req_data(req, struct smb2cli_read_state); - NTSTATUS status; + NTSTATUS status = NT_STATUS_OK; - if (tevent_req_is_nterror(req, &status)) { + if (tevent_req_is_nterror(req, &status) && !state->out_valid) { + *data_length = 0; + *data = NULL; + tevent_req_received(req); return status; } talloc_steal(mem_ctx, state->recv_iov); *data_length = state->data_length; *data = state->data; - return NT_STATUS_OK; + tevent_req_received(req); + return status; } NTSTATUS smb2cli_read(struct smbXcli_conn *conn, -- 1.9.1 From 9f7ce43663380958cc481117911c2c568c80fb8d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 27 Nov 2015 19:10:01 +0100 Subject: [PATCH 2/4] libcli/smb: correctly handle STATUS_BUFFER_OVERFLOW in smb2cli_query_info* BUG: https://bugzilla.samba.org/show_bug.cgi?id=11623 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 68850f3f56e9b28b298c1bc3a6249f9c26602217) --- libcli/smb/smb2cli_query_info.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/libcli/smb/smb2cli_query_info.c b/libcli/smb/smb2cli_query_info.c index a24844b..d499611 100644 --- a/libcli/smb/smb2cli_query_info.c +++ b/libcli/smb/smb2cli_query_info.c @@ -29,6 +29,7 @@ struct smb2cli_query_info_state { uint32_t max_output_length; struct iovec *recv_iov; DATA_BLOB out_output_buffer; + bool out_valid; }; static void smb2cli_query_info_done(struct tevent_req *subreq); @@ -135,8 +136,12 @@ static void smb2cli_query_info_done(struct tevent_req *subreq) status = smb2cli_req_recv(subreq, state, &iov, expected, ARRAY_SIZE(expected)); TALLOC_FREE(subreq); - if (tevent_req_nterror(req, status)) { - return; + if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) { + /* no error */ + } else { + if (tevent_req_nterror(req, status)) { + return; + } } state->recv_iov = iov; @@ -170,6 +175,12 @@ static void smb2cli_query_info_done(struct tevent_req *subreq) state->out_output_buffer.length = output_buffer_length; } + state->out_valid = true; + + if (tevent_req_nterror(req, status)) { + return; + } + tevent_req_done(req); } @@ -180,9 +191,12 @@ NTSTATUS smb2cli_query_info_recv(struct tevent_req *req, struct smb2cli_query_info_state *state = tevent_req_data(req, struct smb2cli_query_info_state); - NTSTATUS status; + NTSTATUS status = NT_STATUS_OK; - if (tevent_req_is_nterror(req, &status)) { + if (tevent_req_is_nterror(req, &status) && !state->out_valid) { + if (out_output_buffer) { + *out_output_buffer = data_blob_null; + } tevent_req_received(req); return status; } @@ -193,7 +207,7 @@ NTSTATUS smb2cli_query_info_recv(struct tevent_req *req, } tevent_req_received(req); - return NT_STATUS_OK; + return status; } NTSTATUS smb2cli_query_info(struct smbXcli_conn *conn, -- 1.9.1 From c6f4757e034cb1fef559c90b7ad433d85e095789 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 27 Nov 2015 19:10:01 +0100 Subject: [PATCH 3/4] libcli/smb: correctly handle STATUS_BUFFER_OVERFLOW in smb1cli_readx* BUG: https://bugzilla.samba.org/show_bug.cgi?id=11623 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 0e8d33fb5ffd6fdb0e503c5ff59e3635bbf10041) --- libcli/smb/smb1cli_read.c | 53 +++++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/libcli/smb/smb1cli_read.c b/libcli/smb/smb1cli_read.c index ab250ab..d7a7f43 100644 --- a/libcli/smb/smb1cli_read.c +++ b/libcli/smb/smb1cli_read.c @@ -26,9 +26,9 @@ struct smb1cli_readx_state { uint32_t size; uint16_t vwv[12]; - NTSTATUS status; uint32_t received; uint8_t *buf; + bool out_valid; }; static void smb1cli_readx_done(struct tevent_req *subreq); @@ -131,27 +131,36 @@ static void smb1cli_readx_done(struct tevent_req *subreq) uint8_t *bytes; uint16_t data_offset; uint32_t bytes_offset; + NTSTATUS status; static const struct smb1cli_req_expected_response expected[] = { { .status = NT_STATUS_OK, .wct = 0x0C }, + { + .status = STATUS_BUFFER_OVERFLOW, + .wct = 0x0C + }, }; - state->status = smb1cli_req_recv(subreq, state, - &recv_iov, - NULL, /* phdr */ - &wct, - &vwv, - NULL, /* pvwv_offset */ - &num_bytes, - &bytes, - &bytes_offset, - NULL, /* inbuf */ - expected, ARRAY_SIZE(expected)); + status = smb1cli_req_recv(subreq, state, + &recv_iov, + NULL, /* phdr */ + &wct, + &vwv, + NULL, /* pvwv_offset */ + &num_bytes, + &bytes, + &bytes_offset, + NULL, /* inbuf */ + expected, ARRAY_SIZE(expected)); TALLOC_FREE(subreq); - if (tevent_req_nterror(req, state->status)) { - return; + if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) { + /* no error */ + } else { + if (tevent_req_nterror(req, status)) { + return; + } } /* size is the number of bytes the server returned. @@ -189,6 +198,12 @@ static void smb1cli_readx_done(struct tevent_req *subreq) state->buf = bytes + (data_offset - bytes_offset); + state->out_valid = true; + + if (tevent_req_nterror(req, status)) { + return; + } + tevent_req_done(req); } @@ -205,7 +220,7 @@ static void smb1cli_readx_done(struct tevent_req *subreq) * @param[out] received The number of bytes received. * @param[out] rcvbuf Pointer to the bytes received. * - * @return NT_STATUS_OK on succsess. + * @return NT_STATUS_OK or STATUS_BUFFER_OVERFLOW on succsess. */ NTSTATUS smb1cli_readx_recv(struct tevent_req *req, uint32_t *received, @@ -213,12 +228,14 @@ NTSTATUS smb1cli_readx_recv(struct tevent_req *req, { struct smb1cli_readx_state *state = tevent_req_data( req, struct smb1cli_readx_state); - NTSTATUS status; + NTSTATUS status = NT_STATUS_OK; - if (tevent_req_is_nterror(req, &status)) { + if (tevent_req_is_nterror(req, &status) && !state->out_valid) { + *received = 0; + *rcvbuf = NULL; return status; } *received = state->received; *rcvbuf = state->buf; - return NT_STATUS_OK; + return status; } -- 1.9.1 From e01d85956811abe30b1c920b8598288c541f3ffd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 27 Nov 2015 18:19:38 +0100 Subject: [PATCH 4/4] libcli/smb: fix BUFFER_OVERFLOW handling in tstream_smbXcli_np The special error is not NT_STATUS_BUFFER_TOO_SMALL, but STATUS_BUFFER_OVERFLOW. Tested using TSTREAM_SMBXCLI_NP_MAX_BUF_SIZE == 20 and running the following commands against a Windows 2012R2 server: bin/smbtorture ncacn_np:SERVER[] rpc.lsa-getuser bin/smbtorture ncacn_np:SERVER[smb2] rpc.lsa-getuser BUG: https://bugzilla.samba.org/show_bug.cgi?id=11623 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Tue Dec 1 03:42:52 CET 2015 on sn-devel-104 (cherry picked from commit 3bbd8d3614af641535ab0925303ad07c03c4e094) --- libcli/smb/tstream_smbXcli_np.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libcli/smb/tstream_smbXcli_np.c b/libcli/smb/tstream_smbXcli_np.c index 248bfb0..a59db13 100644 --- a/libcli/smb/tstream_smbXcli_np.c +++ b/libcli/smb/tstream_smbXcli_np.c @@ -980,7 +980,14 @@ static void tstream_smbXcli_np_readv_trans_done(struct tevent_req *subreq) received = out_output_buffer.length; } TALLOC_FREE(subreq); - if (NT_STATUS_EQUAL(status, NT_STATUS_BUFFER_TOO_SMALL)) { + if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) { + /* + * STATUS_BUFFER_OVERFLOW means that there's + * more data to read when the named pipe is used + * in message mode (which is the case here). + * + * But we hide this from the caller. + */ status = NT_STATUS_OK; } if (!NT_STATUS_IS_OK(status)) { @@ -1056,9 +1063,9 @@ static void tstream_smbXcli_np_readv_read_done(struct tevent_req *subreq) * We can't TALLOC_FREE(subreq) as usual here, as rcvbuf still is a * child of that. */ - if (NT_STATUS_EQUAL(status, NT_STATUS_BUFFER_TOO_SMALL)) { + if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) { /* - * NT_STATUS_BUFFER_TOO_SMALL means that there's + * STATUS_BUFFER_OVERFLOW means that there's * more data to read when the named pipe is used * in message mode (which is the case here). * -- 1.9.1