The branch, master has been updated
       via  76bffc2 Tests processing an oplock break within a compound SMB2 
request.
       via  cbff488 Remove the compound_related_in_progress state from the smb2 
global state.
       via  10cbcfd The core of the fix to allow opens to go async inside a 
compound request.
       via  1102e73 Move a variable into the area of code where it's used.
       via  a026fc6 Ensure we don't try and cancel anything that is in a 
compound-related request.
       via  4111fcf Only do the 1 second delay for sharing violations for SMB1, 
not SMB2.
      from  637887c Makefile: Fix bug 9868 - Don't know how to make 
LIBNDR_PREG_OBJ.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 76bffc27a3f9ad6ac6b8ff8e21f801012835b73d
Author: Richard Sharpe <realrichardsha...@gmail.com>
Date:   Thu May 2 14:36:05 2013 -0700

    Tests processing an oplock break within a compound SMB2 request.
    
    Signed-off-by: Richard Sharpe <realrichardsha...@gmail.com>
    Reviewed-by: Jeremy Allison <j...@samba.org>
    
    Autobuild-User(master): Richard Sharpe <sha...@samba.org>
    Autobuild-Date(master): Tue May  7 19:45:36 CEST 2013 on sn-devel-104

commit cbff4885508e050bcb91c0faccb26941de5c1e1d
Author: Jeremy Allison <j...@samba.org>
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 <j...@samba.org>
    Reviewed-by: Richard Sharpe <realrichardsha...@samba.org>

commit 10cbcfd167a4d7b1a22f9b42b684a66e424cbede
Author: Jeremy Allison <j...@samba.org>
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 <j...@samba.org>
    Reviewed-by: Richard Sharpe <realrichardsha...@gmail.com>

commit 1102e73832f78ca5decc928d6c3649d4fe68eab7
Author: Jeremy Allison <j...@samba.org>
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 <j...@samba.org>
    Reviewed-by: Richard Sharpe <realrichardsha...@gmail.com>

commit a026fc6b699719309a27d4646d06fe1a45b0d158
Author: Jeremy Allison <j...@samba.org>
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 <j...@samba.org>
    Reviewed-by: Richard Sharpe <realrichardsha...@gmail.com>

commit 4111fcfd4f570d39d46a0d414546ca62c7b609be
Author: Jeremy Allison <j...@samba.org>
Date:   Thu May 2 11:12:47 2013 -0700

    Only do the 1 second delay for sharing violations for SMB1, not SMB2.
    
    Match Windows behavior.
    
    Signed-off-by: Jeremy Allison <j...@samba.org>
    Reviewed-by: Richard Sharpe <realrichardsha...@gmail.com>

-----------------------------------------------------------------------

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 0c96eee..cb7630f 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 46baeed..d618aea 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -794,7 +794,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 7d02e52..53f8b8e 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2526,10 +2526,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 57e9c7b..b031c6d 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 4a47e14..9b3cacc 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

Reply via email to