Attachment 14023 Details for Bug 13215 – Patches for v4-6-test

[patch] Patches for v4-6-test

tmp46.diff.txt (text/plain), 16.78 KB, created by Stefan Metzmacher on 2018-03-07 10:56:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2018-03-07 10:56:52 UTC

Size: 16.78 KB

patch

obsolete

>From ded824a1570f676dcacd19761c956114291e939f Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Wed, 10 Jan 2018 14:29:01 +0100
>Subject: [PATCH 1/6] smbd: Fix a typo
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit e8636e7ab75f89e89ef054b5d4aa6c07fddcbe2a)
>---
> source3/smbd/smb2_server.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index de2f922c..900029d 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -2203,7 +2203,7 @@ static NTSTATUS smbd_smb2_request_dispatch_update_counts(
> 		 * a 16 bit overflow of the client-submitted sequence
> 		 * number:
> 		 *
>-		 * If the stored channel squence number is more than
>+		 * If the stored channel sequence number is more than
> 		 * 0x7FFF larger than the one from the request, then
> 		 * the client-provided sequence number has likely
> 		 * overflown. We treat this case as valid instead
>-- 
>1.9.1
>
>
>From 31f608f9d5a64a139de8888924462bd593077675 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Wed, 10 Jan 2018 15:51:56 +0100
>Subject: [PATCH 2/6] torture4: Fix typos
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 71cee27962cba53da3249bd3f5ece32a1d10071d)
>---
> source4/torture/smb2/replay.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
>diff --git a/source4/torture/smb2/replay.c b/source4/torture/smb2/replay.c
>index 26b5583..5c8a372 100644
>--- a/source4/torture/smb2/replay.c
>+++ b/source4/torture/smb2/replay.c
>@@ -473,7 +473,7 @@ done:
> }
> 
> /**
>- * Test Durablity V2 Create Replay Detection on Single Channel.
>+ * Test Durability V2 Create Replay Detection on Single Channel.
>  */
> static bool test_replay_dhv2_oplock1(struct torture_context *tctx,
> 				     struct smb2_tree *tree)
>@@ -560,7 +560,7 @@ done:
> }
> 
> /**
>- * Test Durablity V2 Create Replay Detection on Single Channel.
>+ * Test Durability V2 Create Replay Detection on Single Channel.
>  * Hand in a different oplock level in the replay.
>  * Server responds with the handed in oplock level and
>  * corresponding durable status, but does not change the
>@@ -697,7 +697,7 @@ done:
> }
> 
> /**
>- * Test Durablity V2 Create Replay Detection on Single Channel.
>+ * Test Durability V2 Create Replay Detection on Single Channel.
>  * Replay with a different share mode. The share mode of
>  * the opened file is not changed by this.
>  */
>@@ -823,7 +823,7 @@ done:
> }
> 
> /**
>- * Test Durablity V2 Create Replay Detection on Single Channel.
>+ * Test Durability V2 Create Replay Detection on Single Channel.
>  * Create with an oplock, and replay with a lease.
>  */
> static bool test_replay_dhv2_oplock_lease(struct torture_context *tctx,
>@@ -927,7 +927,7 @@ done:
> 
> 
> /**
>- * Test durablity v2 create replay detection on single channel.
>+ * Test durability v2 create replay detection on single channel.
>  * Variant with leases instead of oplocks:
>  * - open a file with a rh lease
>  * - upgrade to a rwh lease with a second create
>@@ -1065,7 +1065,7 @@ done:
> }
> 
> /**
>- * Test durablity v2 create replay detection on single channel.
>+ * Test durability v2 create replay detection on single channel.
>  * Variant with leases instead of oplocks, where the
>  * replay does not specify the original lease level but
>  * just a "R" lease. This still gives the upgraded lease
>@@ -1216,7 +1216,7 @@ done:
> }
> 
> /**
>- * Test durablity v2 create replay detection on single channel.
>+ * Test durability v2 create replay detection on single channel.
>  * create with a lease, and replay with a different lease key
>  */
> static bool test_replay_dhv2_lease3(struct torture_context *tctx,
>@@ -1349,7 +1349,7 @@ done:
> }
> 
> /**
>- * Test durablity v2 create replay detection on single channel.
>+ * Test durability v2 create replay detection on single channel.
>  * Do the original create with a lease, and do the replay
>  * with an oplock.
>  */
>@@ -1758,7 +1758,7 @@ done:
> }
> 
> /**
>- * Test Durablity V2 Create Replay Detection on Multi Channel
>+ * Test Durability V2 Create Replay Detection on Multi Channel
>  */
> static bool test_replay3(struct torture_context *tctx, struct smb2_tree *tree1)
> {
>@@ -2164,7 +2164,7 @@ done:
> }
> 
> /**
>- * Test Durablity V2 Persistent Create Replay on a Single Channel
>+ * Test Durability V2 Persistent Create Replay on a Single Channel
>  */
> static bool test_replay5(struct torture_context *tctx, struct smb2_tree *tree)
> {
>-- 
>1.9.1
>
>
>From 77b4102f61364ef66e2a663f95690853ea569367 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Wed, 10 Jan 2018 14:59:08 +0100
>Subject: [PATCH 3/6] smbd: Remove a "!" from an if-condition for easier
> readability
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13215
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 03f65a7cdc91091a171269cfebc9916f2f678388)
>---
> source3/smbd/smb2_server.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index 900029d..e3e5199 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -2214,11 +2214,11 @@ static NTSTATUS smbd_smb2_request_dispatch_update_counts(
> 		cmp *= -1;
> 	}
> 
>-	if (!(flags & SMB2_HDR_FLAG_REPLAY_OPERATION)) {
>-		if (cmp == 0) {
>+	if (flags & SMB2_HDR_FLAG_REPLAY_OPERATION) {
>+		if (cmp == 0 && op->pre_request_count == 0) {
> 			op->request_count += 1;
> 			req->request_counters_updated = true;
>-		} else if (cmp > 0) {
>+		} else if (cmp > 0 && op->pre_request_count == 0) {
> 			op->pre_request_count += op->request_count;
> 			op->request_count = 1;
> 			op->global->channel_sequence = channel_sequence;
>@@ -2228,10 +2228,10 @@ static NTSTATUS smbd_smb2_request_dispatch_update_counts(
> 			return NT_STATUS_FILE_NOT_AVAILABLE;
> 		}
> 	} else {
>-		if (cmp == 0 && op->pre_request_count == 0) {
>+		if (cmp == 0) {
> 			op->request_count += 1;
> 			req->request_counters_updated = true;
>-		} else if (cmp > 0 && op->pre_request_count == 0) {
>+		} else if (cmp > 0) {
> 			op->pre_request_count += op->request_count;
> 			op->request_count = 1;
> 			op->global->channel_sequence = channel_sequence;
>-- 
>1.9.1
>
>
>From 0f870b340fec7d7ad6e0cc4878ee7d20430de43c Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Thu, 11 Jan 2018 15:34:45 +0100
>Subject: [PATCH 4/6] smbd: Fix channel sequence number checks for long-running
> requests
>
>When the client's supplied csn overflows and hits a pending, long-running
>request's csn, we panic. Fix this by counting the overflows in
>smbXsrv_open_global0->channel_generation
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13215
>
>Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 0b57434151a8334a6e9b9b7542824ce4915421a2)
>---
> source3/librpc/idl/smbXsrv.idl |  3 ++-
> source3/smbd/globals.h         |  1 +
> source3/smbd/smb2_server.c     | 15 ++++++++++++++-
> 3 files changed, 17 insertions(+), 2 deletions(-)
>
>diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl
>index 1bfa51e..d3f8d30 100644
>--- a/source3/librpc/idl/smbXsrv.idl
>+++ b/source3/librpc/idl/smbXsrv.idl
>@@ -430,7 +430,8 @@ interface smbXsrv
> 		uint32					durable_timeout_msec;
> 		boolean8				durable;
> 		DATA_BLOB				backend_cookie;
>-		hyper					channel_sequence;
>+		uint16					channel_sequence;
>+		hyper					channel_generation;
> 	} smbXsrv_open_global0;
> 
> 	typedef union {
>diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
>index d3b9800..efcf3e9 100644
>--- a/source3/smbd/globals.h
>+++ b/source3/smbd/globals.h
>@@ -733,6 +733,7 @@ struct smbd_smb2_request {
> 	 * adapted again in reply.
> 	 */
> 	bool request_counters_updated;
>+	uint64_t channel_generation;
> 
> 	/*
> 	 * The sub request for async backend calls.
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index e3e5199..573f5f6 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -2141,6 +2141,7 @@ static NTSTATUS smbd_smb2_request_dispatch_update_counts(
> 	struct smbXsrv_connection *xconn = req->xconn;
> 	const uint8_t *inhdr;
> 	uint16_t channel_sequence;
>+	uint8_t generation_wrap = 0;
> 	uint32_t flags;
> 	int cmp;
> 	struct smbXsrv_open *op;
>@@ -2167,6 +2168,14 @@ static NTSTATUS smbd_smb2_request_dispatch_update_counts(
> 	channel_sequence = SVAL(inhdr, SMB2_HDR_CHANNEL_SEQUENCE);
> 
> 	cmp = channel_sequence - op->global->channel_sequence;
>+	if (cmp < 0) {
>+		/*
>+		 * csn wrap. We need to watch out for long-running
>+		 * requests that are still sitting on a previously
>+		 * used csn. SMB2_OP_NOTIFY can take VERY long.
>+		 */
>+		generation_wrap += 1;
>+	}
> 
> 	if (abs(cmp) > INT16_MAX) {
> 		/*
>@@ -2222,6 +2231,7 @@ static NTSTATUS smbd_smb2_request_dispatch_update_counts(
> 			op->pre_request_count += op->request_count;
> 			op->request_count = 1;
> 			op->global->channel_sequence = channel_sequence;
>+			op->global->channel_generation += generation_wrap;
> 			update_open = true;
> 			req->request_counters_updated = true;
> 		} else if (modify_call) {
>@@ -2235,12 +2245,14 @@ static NTSTATUS smbd_smb2_request_dispatch_update_counts(
> 			op->pre_request_count += op->request_count;
> 			op->request_count = 1;
> 			op->global->channel_sequence = channel_sequence;
>+			op->global->channel_generation += generation_wrap;
> 			update_open = true;
> 			req->request_counters_updated = true;
> 		} else if (modify_call) {
> 			return NT_STATUS_FILE_NOT_AVAILABLE;
> 		}
> 	}
>+	req->channel_generation = op->global->channel_generation;
> 
> 	if (update_open) {
> 		status = smbXsrv_open_update(op);
>@@ -2726,7 +2738,8 @@ static void smbd_smb2_request_reply_update_counts(struct smbd_smb2_request *req)
> 	inhdr = SMBD_SMB2_IN_HDR_PTR(req);
> 	channel_sequence = SVAL(inhdr, SMB2_HDR_CHANNEL_SEQUENCE);
> 
>-	if (op->global->channel_sequence == channel_sequence) {
>+	if ((op->global->channel_sequence == channel_sequence) &&
>+	    (op->global->channel_generation == req->channel_generation)) {
> 		SMB_ASSERT(op->request_count > 0);
> 		op->request_count -= 1;
> 	} else {
>-- 
>1.9.1
>
>
>From 7bfc9d910d85c2ee11f6a79d830719c84c6caad6 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Thu, 11 Jan 2018 11:25:49 +0100
>Subject: [PATCH 5/6] smbXcli: Add "force_channel_sequence"
>
>This enables use of the channel sequence number even for
>non-multi-channel servers. This makes our client invalid, but we need to
>protect against broken clients with tests.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13215
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit cd288a08500b1cc38ef26e5cb8ef754b4da658b6)
>---
> libcli/smb/smbXcli_base.c | 15 ++++++++++++++-
> libcli/smb/smbXcli_base.h |  4 ++++
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
>diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
>index 239e5eb..d1e532d 100644
>--- a/libcli/smb/smbXcli_base.c
>+++ b/libcli/smb/smbXcli_base.c
>@@ -138,6 +138,8 @@ struct smbXcli_conn {
> 
> 		uint8_t io_priority;
> 
>+		bool force_channel_sequence;
>+
> 		uint8_t preauth_sha512[64];
> 	} smb2;
> 
>@@ -532,6 +534,17 @@ const struct GUID *smbXcli_conn_server_guid(struct smbXcli_conn *conn)
> 	return &conn->smb1.server.guid;
> }
> 
>+bool smbXcli_conn_get_force_channel_sequence(struct smbXcli_conn *conn)
>+{
>+	return conn->smb2.force_channel_sequence;
>+}
>+
>+void smbXcli_conn_set_force_channel_sequence(struct smbXcli_conn *conn,
>+					     bool v)
>+{
>+	conn->smb2.force_channel_sequence = v;
>+}
>+
> struct smbXcli_conn_samba_suicide_state {
> 	struct smbXcli_conn *conn;
> 	struct iovec iov;
>@@ -2896,7 +2909,7 @@ struct tevent_req *smb2cli_req_create(TALLOC_CTX *mem_ctx,
> 	uint32_t flags = 0;
> 	uint32_t tid = 0;
> 	uint64_t uid = 0;
>-	bool use_channel_sequence = false;
>+	bool use_channel_sequence = conn->smb2.force_channel_sequence;
> 	uint16_t channel_sequence = 0;
> 	bool use_replay_flag = false;
> 
>diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h
>index 2594f07..336b1cbd 100644
>--- a/libcli/smb/smbXcli_base.h
>+++ b/libcli/smb/smbXcli_base.h
>@@ -58,6 +58,10 @@ uint16_t smbXcli_conn_max_requests(struct smbXcli_conn *conn);
> NTTIME smbXcli_conn_server_system_time(struct smbXcli_conn *conn);
> const DATA_BLOB *smbXcli_conn_server_gss_blob(struct smbXcli_conn *conn);
> const struct GUID *smbXcli_conn_server_guid(struct smbXcli_conn *conn);
>+bool smbXcli_conn_get_force_channel_sequence(struct smbXcli_conn *conn);
>+void smbXcli_conn_set_force_channel_sequence(struct smbXcli_conn *conn,
>+					     bool v);
>+
> 
> struct tevent_req *smbXcli_conn_samba_suicide_send(TALLOC_CTX *mem_ctx,
> 						   struct tevent_context *ev,
>-- 
>1.9.1
>
>
>From b272eede7abc47ac37a804d82067662e4f00c5a7 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Thu, 11 Jan 2018 11:55:39 +0100
>Subject: [PATCH 6/6] torture: Add test for channel sequence number handling
>
>We run into an assert when the csn wraps
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13215
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>
>Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
>Autobuild-Date(master): Sun Jan 14 14:47:15 CET 2018 on sn-devel-144
>
>(cherry picked from commit 0abe16a5343de9a69bb5cccbad9809b28b642f45)
>---
> source4/torture/smb2/replay.c | 97 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 97 insertions(+)
>
>diff --git a/source4/torture/smb2/replay.c b/source4/torture/smb2/replay.c
>index 5c8a372..109c04f 100644
>--- a/source4/torture/smb2/replay.c
>+++ b/source4/torture/smb2/replay.c
>@@ -2427,6 +2427,102 @@ done:
> 	return ret;
> }
> 
>+static bool test_replay7(struct torture_context *tctx, struct smb2_tree *tree)
>+{
>+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
>+	struct smb2_transport *transport = tree->session->transport;
>+	NTSTATUS status;
>+	struct smb2_handle _dh;
>+	struct smb2_handle *dh = NULL;
>+	struct smb2_notify notify;
>+	struct smb2_request *req;
>+	union smb_fileinfo qfinfo;
>+	bool ret = false;
>+
>+	if (smbXcli_conn_protocol(transport->conn) < PROTOCOL_SMB3_00) {
>+		torture_skip(tctx, "SMB 3.X Dialect family required for "
>+				   "replay tests\n");
>+	}
>+
>+	torture_comment(tctx, "Notify across increment/decrement of csn\n");
>+
>+	smbXcli_conn_set_force_channel_sequence(transport->conn, true);
>+
>+	status = torture_smb2_testdir(tree, BASEDIR, &_dh);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	dh = &_dh;
>+
>+	notify.in.recursive		= 0x0000;
>+	notify.in.buffer_size	= 0xffff;
>+	notify.in.file.handle	= _dh;
>+	notify.in.completion_filter	= FILE_NOTIFY_CHANGE_FILE_NAME;
>+	notify.in.unknown		= 0x00000000;
>+
>+	/*
>+	 * This posts a long-running request with csn==0 to "dh". Now
>+	 * op->request_count==1 in smb2_server.c.
>+	 */
>+	smb2cli_session_reset_channel_sequence(tree->session->smbXcli, 0);
>+	req = smb2_notify_send(tree, &notify);
>+
>+	qfinfo = (union smb_fileinfo) {
>+		.generic.level = RAW_FILEINFO_POSITION_INFORMATION,
>+		.generic.in.file.handle = _dh
>+	};
>+
>+	/*
>+	 * This sequence of 2 dummy requests moves
>+	 * op->request_count==1 to op->pre_request_count. The numbers
>+	 * used avoid int16 overflow.
>+	 */
>+
>+	smb2cli_session_reset_channel_sequence(tree->session->smbXcli, 30000);
>+	status = smb2_getinfo_file(tree, mem_ctx, &qfinfo);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	smb2cli_session_reset_channel_sequence(tree->session->smbXcli, 60000);
>+	status = smb2_getinfo_file(tree, mem_ctx, &qfinfo);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	/*
>+	 * This final request turns the op->global->channel_sequence
>+	 * to the same as we had when sending the notify above. The
>+	 * notify's request count has in the meantime moved to
>+	 * op->pre_request_count.
>+	 */
>+
>+	smb2cli_session_reset_channel_sequence(tree->session->smbXcli, 0);
>+	status = smb2_getinfo_file(tree, mem_ctx, &qfinfo);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	/*
>+	 * At this point op->request_count==0.
>+	 *
>+	 * The next cancel makes us reply to the notify. Because the
>+	 * csn we currently use is the same as we used when sending
>+	 * the notify, smbd thinks it must decrement op->request_count
>+	 * and not op->pre_request_count.
>+	 */
>+
>+	status = smb2_cancel(req);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	status = smb2_notify_recv(req, mem_ctx, &notify);
>+	CHECK_STATUS(status, NT_STATUS_CANCELLED);
>+
>+	ret = true;
>+
>+done:
>+	if (dh != NULL) {
>+		smb2_util_close(tree, _dh);
>+	}
>+	smb2_deltree(tree, BASEDIR);
>+	talloc_free(tree);
>+	talloc_free(mem_ctx);
>+
>+	return ret;
>+}
>+
> struct torture_suite *torture_smb2_replay_init(void)
> {
> 	struct torture_suite *suite =
>@@ -2447,6 +2543,7 @@ struct torture_suite *torture_smb2_replay_init(void)
> 	torture_suite_add_1smb2_test(suite, "replay4", test_replay4);
> 	torture_suite_add_1smb2_test(suite, "replay5", test_replay5);
> 	torture_suite_add_1smb2_test(suite, "replay6", test_replay6);
>+	torture_suite_add_1smb2_test(suite, "replay7", test_replay7);
> 
> 	suite->description = talloc_strdup(suite, "SMB2 REPLAY tests");
> 
>-- 
>1.9.1
>

Flags:

slow: review+
metze: review? (jra)

Actions: View

Attachments on bug 13215: 14022 | 14023 | 14122 | 14123 | 14127 | 14128 | 14129