[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Thu May 28 08:51:04 MDT 2015


The branch, master has been updated
       via  2ffa939 s4:torture:smb2:compound: compound read and padding
       via  dfa64b9 s3:smb2: add padding to last command in compound requests
      from  a4cc7d4 messages_ctdb: Use message_hdr_[get/put]

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


- Log -----------------------------------------------------------------
commit 2ffa939bbe2c02509e1790c8b3f6f9b6910e3cf6
Author: Ralph Boehme <slow at samba.org>
Date:   Thu May 14 04:27:54 2015 +0200

    s4:torture:smb2:compound: compound read and padding
    
    Add test to check that compound read responses are padded to an 8 byte
    boundary.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=11277
    
    Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Thu May 28 16:50:39 CEST 2015 on sn-devel-104

commit dfa64b958b201931e0dbe11f153f606f20217594
Author: Ralph Boehme <slow at samba.org>
Date:   Thu May 28 09:02:17 2015 +0200

    s3:smb2: add padding to last command in compound requests
    
    Following Windows behaviour, the last command in a compound request
    should be padded to an 8 byte boundary and OS X clients crash badly if
    we don't pad.
    
    [MS-SMB2] 3.3.4.1.3, "Sending Compounded Responses", doesn't make it
    clear whether the padding requirement governs the last command in a
    compound response, a future MS-SMB2 update will document Windwows
    product behaviour in a footnote.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=11277
    
    Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Signed-off-by: Stefan Metzmacher <metze at samba.org>

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

Summary of changes:
 source3/smbd/smb2_server.c      |  16 ++-
 source4/torture/smb2/compound.c | 239 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 251 insertions(+), 4 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 9e5eff7..38590a9 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -2654,8 +2654,13 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req,
 		outdyn_v->iov_len = 0;
 	}
 
-	/* see if we need to recalculate the offset to the next response */
-	if (next_command_ofs > 0) {
+	/*
+	 * See if we need to recalculate the offset to the next response
+	 *
+	 * Note that all responses may require padding (including the very last
+	 * one).
+	 */
+	if (req->out.vector_count >= (2 * SMBD_SMB2_NUM_IOV_PER_REQ)) {
 		next_command_ofs  = SMB2_HDR_BODY;
 		next_command_ofs += SMBD_SMB2_OUT_BODY_LEN(req);
 		next_command_ofs += SMBD_SMB2_OUT_DYN_LEN(req);
@@ -2709,8 +2714,11 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req,
 		next_command_ofs += pad_size;
 	}
 
-	SIVAL(outhdr, SMB2_HDR_NEXT_COMMAND, next_command_ofs);
-
+	if ((req->current_idx + SMBD_SMB2_NUM_IOV_PER_REQ) >= req->out.vector_count) {
+		SIVAL(outhdr, SMB2_HDR_NEXT_COMMAND, 0);
+	} else {
+		SIVAL(outhdr, SMB2_HDR_NEXT_COMMAND, next_command_ofs);
+	}
 	return smbd_smb2_request_reply(req);
 }
 
diff --git a/source4/torture/smb2/compound.c b/source4/torture/smb2/compound.c
index 9b3cacc..a502103 100644
--- a/source4/torture/smb2/compound.c
+++ b/source4/torture/smb2/compound.c
@@ -34,6 +34,14 @@
 		goto done; \
 	}} while (0)
 
+#define CHECK_VALUE(v, correct) do { \
+	if ((v) != (correct)) { \
+		torture_result(tctx, TORTURE_FAIL, \
+		    "(%s) Incorrect value %s=%d - should be %d\n", \
+		    __location__, #v, (int)v, (int)correct); \
+		ret = false; \
+	}} while (0)
+
 static struct {
 	struct smb2_handle handle;
 	uint8_t level;
@@ -433,6 +441,236 @@ done:
 	return ret;
 }
 
+static bool test_compound_padding(struct torture_context *tctx,
+				  struct smb2_tree *tree)
+{
+	struct smb2_handle h;
+	struct smb2_create cr;
+	struct smb2_read r;
+	const char *fname = "compound_read.dat";
+	const char *sname = "compound_read.dat:foo";
+	struct smb2_request *req[3];
+	NTSTATUS status;
+	bool ret = false;
+
+	smb2_util_unlink(tree, fname);
+
+	/* Write file */
+	ZERO_STRUCT(cr);
+	cr.in.desired_access = SEC_FILE_WRITE_DATA;
+	cr.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+	cr.in.create_disposition = NTCREATEX_DISP_CREATE;
+	cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	cr.in.fname = fname;
+	cr.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+		NTCREATEX_SHARE_ACCESS_WRITE|
+		NTCREATEX_SHARE_ACCESS_DELETE;
+	status = smb2_create(tree, tctx, &cr);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h = cr.out.file.handle;
+
+	status = smb2_util_write(tree, h, "123", 0, 3);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	smb2_util_close(tree, h);
+
+	/* Write stream */
+	ZERO_STRUCT(cr);
+	cr.in.desired_access = SEC_FILE_WRITE_DATA;
+	cr.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+	cr.in.create_disposition = NTCREATEX_DISP_CREATE;
+	cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	cr.in.fname = sname;
+	cr.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+		NTCREATEX_SHARE_ACCESS_WRITE|
+		NTCREATEX_SHARE_ACCESS_DELETE;
+	status = smb2_create(tree, tctx, &cr);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h = cr.out.file.handle;
+
+	status = smb2_util_write(tree, h, "456", 0, 3);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	smb2_util_close(tree, h);
+
+	/* Check compound read from basefile */
+	smb2_transport_compound_start(tree->session->transport, 2);
+
+	ZERO_STRUCT(cr);
+	cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	cr.in.desired_access	= SEC_FILE_READ_DATA;
+	cr.in.file_attributes	= FILE_ATTRIBUTE_NORMAL;
+	cr.in.create_disposition = NTCREATEX_DISP_OPEN;
+	cr.in.fname		= fname;
+	cr.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+		NTCREATEX_SHARE_ACCESS_WRITE|
+		NTCREATEX_SHARE_ACCESS_DELETE;
+	req[0] = smb2_create_send(tree, &cr);
+
+	smb2_transport_compound_set_related(tree->session->transport, true);
+
+	ZERO_STRUCT(r);
+	h.data[0] = UINT64_MAX;
+	h.data[1] = UINT64_MAX;
+	r.in.file.handle = h;
+	r.in.length      = 3;
+	r.in.offset      = 0;
+	r.in.min_count      = 1;
+	req[1] = smb2_read_send(tree, &r);
+
+	status = smb2_create_recv(req[0], tree, &cr);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/*
+	 * We must do a manual smb2_request_receive() in order to be
+	 * able to check the transport layer info, as smb2_read_recv()
+	 * will destroy the req. smb2_read_recv() will call
+	 * smb2_request_receive() again, but that's ok.
+	 */
+	if (!smb2_request_receive(req[1]) ||
+	    !smb2_request_is_ok(req[1])) {
+		torture_fail(tctx, "failed to receive read request");
+	}
+
+	/*
+	 * size must be 24: 16 byte read response header plus 3
+	 * requested bytes padded to an 8 byte boundary.
+	 */
+	CHECK_VALUE(req[1]->in.body_size, 24);
+
+	status = smb2_read_recv(req[1], tree, &r);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	smb2_util_close(tree, cr.out.file.handle);
+
+	/* Check compound read from stream */
+	smb2_transport_compound_start(tree->session->transport, 2);
+
+	ZERO_STRUCT(cr);
+	cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	cr.in.desired_access	= SEC_FILE_READ_DATA;
+	cr.in.file_attributes	= FILE_ATTRIBUTE_NORMAL;
+	cr.in.create_disposition = NTCREATEX_DISP_OPEN;
+	cr.in.fname		= sname;
+	cr.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+		NTCREATEX_SHARE_ACCESS_WRITE|
+		NTCREATEX_SHARE_ACCESS_DELETE;
+	req[0] = smb2_create_send(tree, &cr);
+
+	smb2_transport_compound_set_related(tree->session->transport, true);
+
+	ZERO_STRUCT(r);
+	h.data[0] = UINT64_MAX;
+	h.data[1] = UINT64_MAX;
+	r.in.file.handle = h;
+	r.in.length      = 3;
+	r.in.offset      = 0;
+	r.in.min_count   = 1;
+	req[1] = smb2_read_send(tree, &r);
+
+	status = smb2_create_recv(req[0], tree, &cr);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/*
+	 * We must do a manual smb2_request_receive() in order to be
+	 * able to check the transport layer info, as smb2_read_recv()
+	 * will destroy the req. smb2_read_recv() will call
+	 * smb2_request_receive() again, but that's ok.
+	 */
+	if (!smb2_request_receive(req[1]) ||
+	    !smb2_request_is_ok(req[1])) {
+		torture_fail(tctx, "failed to receive read request");
+	}
+
+	/*
+	 * size must be 24: 16 byte read response header plus 3
+	 * requested bytes padded to an 8 byte boundary.
+	 */
+	CHECK_VALUE(req[1]->in.body_size, 24);
+
+	status = smb2_read_recv(req[1], tree, &r);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	h = cr.out.file.handle;
+
+	/* Check 2 compound (unrelateated) reads from existing stream handle */
+	smb2_transport_compound_start(tree->session->transport, 2);
+
+	ZERO_STRUCT(r);
+	r.in.file.handle = h;
+	r.in.length      = 3;
+	r.in.offset      = 0;
+	r.in.min_count   = 1;
+	req[0] = smb2_read_send(tree, &r);
+	req[1] = smb2_read_send(tree, &r);
+
+	/*
+	 * We must do a manual smb2_request_receive() in order to be
+	 * able to check the transport layer info, as smb2_read_recv()
+	 * will destroy the req. smb2_read_recv() will call
+	 * smb2_request_receive() again, but that's ok.
+	 */
+	if (!smb2_request_receive(req[0]) ||
+	    !smb2_request_is_ok(req[0])) {
+		torture_fail(tctx, "failed to receive read request");
+	}
+	if (!smb2_request_receive(req[1]) ||
+	    !smb2_request_is_ok(req[1])) {
+		torture_fail(tctx, "failed to receive read request");
+	}
+
+	/*
+	 * size must be 24: 16 byte read response header plus 3
+	 * requested bytes padded to an 8 byte boundary.
+	 */
+	CHECK_VALUE(req[0]->in.body_size, 24);
+	CHECK_VALUE(req[1]->in.body_size, 24);
+
+	status = smb2_read_recv(req[0], tree, &r);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	status = smb2_read_recv(req[1], tree, &r);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/*
+	 * now try a single read from the stream and verify there's no padding
+	 */
+	ZERO_STRUCT(r);
+	r.in.file.handle = h;
+	r.in.length      = 3;
+	r.in.offset      = 0;
+	r.in.min_count   = 1;
+	req[0] = smb2_read_send(tree, &r);
+
+	/*
+	 * We must do a manual smb2_request_receive() in order to be
+	 * able to check the transport layer info, as smb2_read_recv()
+	 * will destroy the req. smb2_read_recv() will call
+	 * smb2_request_receive() again, but that's ok.
+	 */
+	if (!smb2_request_receive(req[0]) ||
+	    !smb2_request_is_ok(req[0])) {
+		torture_fail(tctx, "failed to receive read request");
+	}
+
+	/*
+	 * size must be 19: 16 byte read response header plus 3
+	 * requested bytes without padding.
+	 */
+	CHECK_VALUE(req[0]->in.body_size, 19);
+
+	status = smb2_read_recv(req[0], tree, &r);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	smb2_util_close(tree, h);
+
+	status = smb2_util_unlink(tree, fname);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	ret = true;
+done:
+	return ret;
+}
+
 static bool test_compound_unrelated1(struct torture_context *tctx,
 				     struct smb2_tree *tree)
 {
@@ -880,6 +1118,7 @@ struct torture_suite *torture_smb2_compound_init(void)
 	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);
+	torture_suite_add_1smb2_test(suite, "compound-padding", test_compound_padding);
 
 	suite->description = talloc_strdup(suite, "SMB2-COMPOUND tests");
 


-- 
Samba Shared Repository


More information about the samba-cvs mailing list