The branch, v4-0-test has been updated
via cd9be74 Tests processing an oplock break within a compound SMB2
request.
via 457af02 Remove the compound_related_in_progress state from the smb2
global state.
via 5ab0457 The core of the fix to allow opens to go async inside a
compound request.
via 805b029 Move a variable into the area of code where it's used.
via 0fea4e3 Ensure we don't try and cancel anything that is in a
compound-related request.
via 0770400 Only do the 1 second delay for sharing violations for SMB1,
not SMB2.
from 23a75ad winbind: Fix bug 9854 -- NULL pointer dereference
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v4-0-test
- Log -----------------------------------------------------------------
commit cd9be74acbb5e09c5304516fa31cbb0b64da7d12
Author: Richard Sharpe <[email protected]>
Date: Thu May 2 14:36:05 2013 -0700
Tests processing an oplock break within a compound SMB2 request.
Signed-off-by: Richard Sharpe <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
Autobuild-User(master): Richard Sharpe <[email protected]>
Autobuild-Date(master): Tue May 7 19:45:36 CEST 2013 on sn-devel-104
(cherry picked from commit 76bffc27a3f9ad6ac6b8ff8e21f801012835b73d)
The last 6 patches address bug #9722 - Samba does not properly handle Oplock
breaks in compound requests.
Autobuild-User(v4-0-test): Karolin Seeger <[email protected]>
Autobuild-Date(v4-0-test): Wed May 8 23:11:55 CEST 2013 on sn-devel-104
commit 457af02a687ff479fdcc7d870db3648466111c64
Author: Jeremy Allison <[email protected]>
Date: Thu May 2 14:16:22 2013 -0700
Remove the compound_related_in_progress state from the smb2 global state.
And also remove the restriction that we can't read a new
request whilst we're in this state.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Richard Sharpe <[email protected]>
(cherry picked from commit cbff4885508e050bcb91c0faccb26941de5c1e1d)
commit 5ab0457fec6fc7816fb7007e18583f1a471a28f1
Author: Jeremy Allison <[email protected]>
Date: Thu May 2 13:55:53 2013 -0700
The core of the fix to allow opens to go async inside a compound request.
This is only allowed for opens that cause an oplock break, otherwise it
is not allowed. See [MS-SMB2].pdf note <194> on Section 3.3.5.2.7.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Richard Sharpe <[email protected]>
(cherry picked from commit 10cbcfd167a4d7b1a22f9b42b684a66e424cbede)
commit 805b0296ff8c87bf6b340ce199f4da2ab66c8654
Author: Jeremy Allison <[email protected]>
Date: Thu May 2 13:08:16 2013 -0700
Move a variable into the area of code where it's used.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Richard Sharpe <[email protected]>
(cherry picked from commit 1102e73832f78ca5decc928d6c3649d4fe68eab7)
commit 0fea4e392a37b884242c26a35990ab6313b121a0
Author: Jeremy Allison <[email protected]>
Date: Thu May 2 12:34:54 2013 -0700
Ensure we don't try and cancel anything that is in a compound-related
request.
Too hard to deal with splitting off the replies.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Richard Sharpe <[email protected]>
(cherry picked from commit a026fc6b699719309a27d4646d06fe1a45b0d158)
commit 07704006805108641f2391094432d52f0aa78098
Author: Jeremy Allison <[email protected]>
Date: Tue May 7 12:07:16 2013 -0700
Only do the 1 second delay for sharing violations for SMB1, not SMB2.
Match Windows behavior.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Richard Sharpe <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
selftest/knownfail | 1 +
source3/smbd/globals.h | 1 -
source3/smbd/open.c | 3 +-
source3/smbd/smb2_server.c | 115 +++++++++++++++-------------
source4/torture/smb2/compound.c | 163 +++++++++++++++++++++++++++++++++++++++
5 files changed, 227 insertions(+), 56 deletions(-)
Changeset truncated at 500 lines:
diff --git a/selftest/knownfail b/selftest/knownfail
index e3964d6..262b889 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -107,6 +107,7 @@
^samba4.smb2.rename.no_share_delete_no_delete_access\(.*\)$
^samba4.smb2.rename.msword
^samba4.smb2.compound.related3
+^samba4.smb2.compound.compound-break
^samba4.winbind.struct.*.show_sequence # Not yet working in winbind
^samba4.*base.delaywrite.*update of write time and SMBwrite truncate\(.*\)$
^samba4.*base.delaywrite.*update of write time and SMBwrite truncate
expand\(.*\)$
diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 16ac24b..1182ae9 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -786,7 +786,6 @@ struct smbd_server_connection {
uint32_t max_trans;
uint32_t max_read;
uint32_t max_write;
- bool compound_related_in_progress;
} smb2;
struct smbXsrv_connection *conn;
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index c5529ec..16ca34a 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2345,10 +2345,11 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
/*
* If we're returning a share violation, ensure we
- * cope with the braindead 1 second delay.
+ * cope with the braindead 1 second delay (SMB1 only).
*/
if (!(oplock_request & INTERNAL_OPEN_ONLY) &&
+ !conn->sconn->using_smb2 &&
lp_defer_sharing_violations()) {
struct timeval timeout;
struct deferred_open_record state;
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 29d4f7c..525b81b 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -1272,7 +1272,6 @@ NTSTATUS smbd_smb2_request_pending_queue(struct
smbd_smb2_request *req,
uint32_t defer_time)
{
NTSTATUS status;
- int idx = req->current_idx;
struct timeval defer_endtime;
uint8_t *outhdr = NULL;
uint32_t flags;
@@ -1296,19 +1295,29 @@ NTSTATUS smbd_smb2_request_pending_queue(struct
smbd_smb2_request *req,
return NT_STATUS_OK;
}
- if (req->in.vector_count > idx + SMBD_SMB2_NUM_IOV_PER_REQ) {
+ if (req->in.vector_count > req->current_idx +
SMBD_SMB2_NUM_IOV_PER_REQ) {
/*
* We're trying to go async in a compound
- * request chain. This is not allowed.
- * Cancel the outstanding request.
+ * request chain.
+ * This is only allowed for opens that
+ * cause an oplock break, otherwise it
+ * is not allowed. See [MS-SMB2].pdf
+ * note <194> on Section 3.3.5.2.7.
*/
- bool ok = tevent_req_cancel(req->subreq);
- if (ok) {
- return NT_STATUS_OK;
+ const uint8_t *inhdr = SMBD_SMB2_IN_HDR_PTR(req);
+
+ if (SVAL(inhdr, SMB2_HDR_OPCODE) != SMB2_OP_CREATE) {
+ /*
+ * Cancel the outstanding request.
+ */
+ bool ok = tevent_req_cancel(req->subreq);
+ if (ok) {
+ return NT_STATUS_OK;
+ }
+ TALLOC_FREE(req->subreq);
+ return smbd_smb2_request_error(req,
+ NT_STATUS_INTERNAL_ERROR);
}
- TALLOC_FREE(req->subreq);
- return smbd_smb2_request_error(req,
- NT_STATUS_INTERNAL_ERROR);
}
if (DEBUGLEVEL >= 10) {
@@ -1317,50 +1326,50 @@ NTSTATUS smbd_smb2_request_pending_queue(struct
smbd_smb2_request *req,
print_req_vectors(req);
}
- if (req->out.vector_count >= (2*SMBD_SMB2_NUM_IOV_PER_REQ)) {
+ if (req->current_idx > 1) {
/*
- * This is a compound reply. We
- * must do an interim response
- * followed by the async response
- * to match W2K8R2.
+ * We're going async in a compound
+ * chain after the first request has
+ * already been processed. Send an
+ * interim response containing the
+ * set of replies already generated.
*/
+ int idx = req->current_idx;
+
status = smb2_send_async_interim_response(req);
if (!NT_STATUS_IS_OK(status)) {
return status;
}
data_blob_clear_free(&req->first_key);
- /*
- * We're splitting off the last SMB2
- * request in a compound set, and the
- * smb2_send_async_interim_response()
- * call above just sent all the replies
- * for the previous SMB2 requests in
- * this compound set. So we're no longer
- * in the "compound_related_in_progress"
- * state, and this is no longer a compound
- * request.
- */
- req->compound_related = false;
- req->sconn->smb2.compound_related_in_progress = false;
-
req->current_idx = 1;
- /* Re-arrange the in.vectors. */
- memmove(&req->in.vector[req->current_idx],
- &req->in.vector[idx],
- sizeof(req->in.vector[0])*SMBD_SMB2_NUM_IOV_PER_REQ);
- req->in.vector_count = req->current_idx +
SMBD_SMB2_NUM_IOV_PER_REQ;
-
- /* Re-arrange the out.vectors. */
- memmove(&req->out.vector[req->current_idx],
- &req->out.vector[idx],
- sizeof(req->out.vector[0])*SMBD_SMB2_NUM_IOV_PER_REQ);
- req->out.vector_count = req->current_idx +
SMBD_SMB2_NUM_IOV_PER_REQ;
-
- outhdr = SMBD_SMB2_OUT_HDR_PTR(req);
- flags = (IVAL(outhdr, SMB2_HDR_FLAGS) & ~SMB2_HDR_FLAG_CHAINED);
- SIVAL(outhdr, SMB2_HDR_FLAGS, flags);
+ /*
+ * Re-arrange the in.vectors to remove what
+ * we just sent.
+ */
+ memmove(&req->in.vector[1],
+ &req->in.vector[idx],
+ sizeof(req->in.vector[0])*(req->in.vector_count - idx));
+ req->in.vector_count = 1 + (req->in.vector_count - idx);
+
+ /* Re-arrange the out.vectors to match. */
+ memmove(&req->out.vector[1],
+ &req->out.vector[idx],
+ sizeof(req->out.vector[0])*(req->out.vector_count -
idx));
+ req->out.vector_count = 1 + (req->out.vector_count - idx);
+
+ if (req->in.vector_count == 1 + SMBD_SMB2_NUM_IOV_PER_REQ) {
+ /*
+ * We only have one remaining request as
+ * we've processed everything else.
+ * This is no longer a compound request.
+ */
+ req->compound_related = false;
+ outhdr = SMBD_SMB2_OUT_HDR_PTR(req);
+ flags = (IVAL(outhdr, SMB2_HDR_FLAGS) &
~SMB2_HDR_FLAG_CHAINED);
+ SIVAL(outhdr, SMB2_HDR_FLAGS, flags);
+ }
}
data_blob_clear_free(&req->last_key);
@@ -1599,6 +1608,14 @@ static NTSTATUS smbd_smb2_request_process_cancel(struct
smbd_smb2_request *req)
uint64_t message_id;
uint64_t async_id;
+ if (cur->compound_related) {
+ /*
+ * Never cancel anything in a compound request.
+ * Way too hard to deal with the result.
+ */
+ continue;
+ }
+
outhdr = SMBD_SMB2_OUT_HDR_PTR(cur);
message_id = BVAL(outhdr, SMB2_HDR_MESSAGE_ID);
@@ -2024,7 +2041,6 @@ NTSTATUS smbd_smb2_request_dispatch(struct
smbd_smb2_request *req)
if (flags & SMB2_HDR_FLAG_CHAINED) {
req->compound_related = true;
- req->sconn->smb2.compound_related_in_progress = true;
}
if (call->need_session) {
@@ -2388,7 +2404,6 @@ static NTSTATUS smbd_smb2_request_reply(struct
smbd_smb2_request *req)
if (req->compound_related) {
req->compound_related = false;
- req->sconn->smb2.compound_related_in_progress = false;
}
smb2_setup_nbt_length(req->out.vector, req->out.vector_count);
@@ -3143,14 +3158,6 @@ static NTSTATUS smbd_smb2_request_next_incoming(struct
smbd_server_connection *s
size_t cur_send_queue_len;
struct tevent_req *subreq;
- if (sconn->smb2.compound_related_in_progress) {
- /*
- * Can't read another until the related
- * compound is done.
- */
- return NT_STATUS_OK;
- }
-
if (tevent_queue_length(sconn->smb2.recv_queue) > 0) {
/*
* if there is already a smbd_smb2_request_read
diff --git a/source4/torture/smb2/compound.c b/source4/torture/smb2/compound.c
index e75f682..132f94f 100644
--- a/source4/torture/smb2/compound.c
+++ b/source4/torture/smb2/compound.c
@@ -34,6 +34,168 @@
goto done; \
}} while (0)
+static struct {
+ struct smb2_handle handle;
+ uint8_t level;
+ struct smb2_break br;
+ int count;
+ int failures;
+ NTSTATUS failure_status;
+} break_info;
+
+static void torture_oplock_break_callback(struct smb2_request *req)
+{
+ NTSTATUS status;
+ struct smb2_break br;
+
+ ZERO_STRUCT(br);
+ status = smb2_break_recv(req, &break_info.br);
+ if (!NT_STATUS_IS_OK(status)) {
+ break_info.failures++;
+ break_info.failure_status = status;
+ }
+
+ return;
+}
+
+/* A general oplock break notification handler. This should be used when a
+ * test expects to break from batch or exclusive to a lower level. */
+static bool torture_oplock_handler(struct smb2_transport *transport,
+ const struct smb2_handle *handle,
+ uint8_t level,
+ void *private_data)
+{
+ struct smb2_tree *tree = private_data;
+ const char *name;
+ struct smb2_request *req;
+ ZERO_STRUCT(break_info.br);
+
+ break_info.handle = *handle;
+ break_info.level = level;
+ break_info.count++;
+
+ switch (level) {
+ case SMB2_OPLOCK_LEVEL_II:
+ name = "level II";
+ break;
+ case SMB2_OPLOCK_LEVEL_NONE:
+ name = "none";
+ break;
+ default:
+ name = "unknown";
+ break_info.failures++;
+ }
+ printf("Acking to %s [0x%02X] in oplock handler\n", name, level);
+
+ break_info.br.in.file.handle = *handle;
+ break_info.br.in.oplock_level = level;
+ break_info.br.in.reserved = 0;
+ break_info.br.in.reserved2 = 0;
+
+ req = smb2_break_send(tree, &break_info.br);
+ req->async.fn = torture_oplock_break_callback;
+ req->async.private_data = NULL;
+ return true;
+}
+
+static bool test_compound_break(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ const char *fname1 = "some-file.pptx";
+ NTSTATUS status;
+ bool ret = true;
+ union smb_open io1;
+ struct smb2_create io2;
+ struct smb2_getinfo gf;
+ struct smb2_request *req[2];
+ struct smb2_handle h1;
+ struct smb2_handle h;
+
+ tree->session->transport->oplock.handler = torture_oplock_handler;
+ tree->session->transport->oplock.private_data = tree;
+
+ ZERO_STRUCT(break_info);
+
+ /*
+ base ntcreatex parms
+ */
+ ZERO_STRUCT(io1.smb2);
+ io1.generic.level = RAW_OPEN_SMB2;
+ io1.smb2.in.desired_access = (SEC_STD_SYNCHRONIZE|
+ SEC_STD_READ_CONTROL|
+ SEC_FILE_READ_ATTRIBUTE|
+ SEC_FILE_READ_EA|
+ SEC_FILE_READ_DATA);
+ io1.smb2.in.alloc_size = 0;
+ io1.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+ io1.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+ NTCREATEX_SHARE_ACCESS_WRITE|
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ io1.smb2.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
+ io1.smb2.in.create_options = 0;
+ io1.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+ io1.smb2.in.security_flags = 0;
+ io1.smb2.in.fname = fname1;
+
+ torture_comment(tctx, "TEST2: open a file with an batch "
+ "oplock (share mode: all)\n");
+ io1.smb2.in.oplock_level = SMB2_OPLOCK_LEVEL_BATCH;
+
+ status = smb2_create(tree, tctx, &(io1.smb2));
+ torture_assert_ntstatus_ok(tctx, status, "Error opening the file");
+
+ h1 = io1.smb2.out.file.handle;
+
+ torture_comment(tctx, "TEST2: Opening second time with compound\n");
+
+ ZERO_STRUCT(io2);
+
+ io2.in.desired_access = (SEC_STD_SYNCHRONIZE|
+ SEC_FILE_READ_ATTRIBUTE|
+ SEC_FILE_READ_EA);
+ io2.in.alloc_size = 0;
+ io2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+ io2.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+ NTCREATEX_SHARE_ACCESS_WRITE|
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ io2.in.create_disposition = NTCREATEX_DISP_OPEN;
+ io2.in.create_options = 0;
+ io2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+ io2.in.security_flags = 0;
+ io2.in.fname = fname1;
+ io2.in.oplock_level = 0;
+
+ smb2_transport_compound_start(tree->session->transport, 2);
+
+ req[0] = smb2_create_send(tree, &io2);
+
+ smb2_transport_compound_set_related(tree->session->transport, true);
+
+ h.data[0] = UINT64_MAX;
+ h.data[1] = UINT64_MAX;
+
+ ZERO_STRUCT(gf);
+ gf.in.file.handle = h;
+ gf.in.info_type = SMB2_GETINFO_FILE;
+ gf.in.info_class = 0x16;
+ gf.in.output_buffer_length = 0x1000;
+ gf.in.input_buffer_length = 0;
+
+ req[1] = smb2_getinfo_send(tree, &gf);
+
+ status = smb2_create_recv(req[0], tree, &io2);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ status = smb2_getinfo_recv(req[1], tree, &gf);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+done:
+
+ smb2_util_close(tree, h1);
+ smb2_util_unlink(tree, fname1);
+ return ret;
+}
+
static bool test_compound_related1(struct torture_context *tctx,
struct smb2_tree *tree)
{
@@ -717,6 +879,7 @@ struct torture_suite *torture_smb2_compound_init(void)
torture_suite_add_1smb2_test(suite, "invalid3", test_compound_invalid3);
torture_suite_add_1smb2_test(suite, "interim1",
test_compound_interim1);
torture_suite_add_1smb2_test(suite, "interim2",
test_compound_interim2);
+ torture_suite_add_1smb2_test(suite, "compound-break",
test_compound_break);
suite->description = talloc_strdup(suite, "SMB2-COMPOUND tests");
--
Samba Shared Repository
