Attachment 11099 Details for Bug 11277 – Patch for master

[patch] Patch for master

compound.patch (text/plain), 8.15 KB, created by Ralph Böhme on 2015-05-28 08:50:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2015-05-28 08:50:59 UTC

Size: 8.15 KB

patch

obsolete

>From 314e0031a76ee1f169d2788cb97fe2452fd0f012 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Thu, 28 May 2015 09:02:17 +0200
>Subject: [PATCH 1/2] 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
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>---
> source3/smbd/smb2_server.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index 9e5eff7..1ef3034 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -2655,7 +2655,7 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req,
> 	}
> 
> 	/* see if we need to recalculate the offset to the next response */
>-	if (next_command_ofs > 0) {
>+	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 +2709,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);
> }
> 
>-- 
>2.1.0
>
>
>From 260c6b5c4b636342bff10929f9f30b029d87458b Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Thu, 14 May 2015 04:27:54 +0200
>Subject: [PATCH 2/2] 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
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>---
> source4/torture/smb2/compound.c | 168 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 168 insertions(+)
>
>diff --git a/source4/torture/smb2/compound.c b/source4/torture/smb2/compound.c
>index 9b3cacc..7eb1ed2 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,165 @@ 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);
>+
>+	smb2_util_close(tree, cr.out.file.handle);
>+	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 +1047,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");
> 
>-- 
>2.1.0
>

Actions: View

Attachments on bug 11277: 11057 | 11058 | 11063 | 11099 | 11103 | 11104