Attachment 9707 Details for Bug 10444 – Patch for v4-1-test

[patch] Patch for v4-1-test

tmp41.diff (text/plain), 12.98 KB, created by Stefan Metzmacher on 2014-02-21 13:56:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2014-02-21 13:56:11 UTC

Size: 12.98 KB

patch

obsolete

>From e1daeec874ab7ce982a2af7b1d5f0ab6aaf8e996 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 17 Feb 2014 12:05:42 +0100
>Subject: [PATCH 1/4] s3:smbd: simplify exit_server_common()
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit 58c71bee40bb91868fc69d8f7fa640db0e33efae)
>---
> source3/smbd/server_exit.c |    6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/source3/smbd/server_exit.c b/source3/smbd/server_exit.c
>index dfa2b1d..f99b9a4 100644
>--- a/source3/smbd/server_exit.c
>+++ b/source3/smbd/server_exit.c
>@@ -99,13 +99,11 @@ static void exit_server_common(enum server_exit_reason how,
> 
> 	change_to_root_user();
> 
>-	if (sconn && sconn->smb1.negprot.auth_context) {
>-		TALLOC_FREE(sconn->smb1.negprot.auth_context);
>-	}
>-
> 	if (sconn) {
> 		NTSTATUS status;
> 
>+		TALLOC_FREE(sconn->smb1.negprot.auth_context);
>+
> 		if (lp_log_writeable_files_on_exit()) {
> 			bool found = false;
> 			files_forall(sconn, log_writeable_file_fn, &found);
>-- 
>1.7.9.5
>
>
>From a8cd461bd5f2535acea5d7a34c25d02815033a6a Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 17 Feb 2014 12:01:12 +0100
>Subject: [PATCH 2/4] s3:smbd: maintain smbd_server_connection->status
>
>If this isn't NT_STATUS_OK, we skip any io on the socket.
>
>This avoids possible problems during shutdown.
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit 52ccb40d595fc80bfa53b0b9cd75ffb902369681)
>---
> source3/smbd/globals.h     |    1 +
> source3/smbd/process.c     |   25 +++++++++++++++++++++++++
> source3/smbd/server_exit.c |   11 +++++++++++
> source3/smbd/smb2_server.c |   16 ++++++++++++++++
> 4 files changed, 53 insertions(+)
>
>diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
>index df0e524..1286ced 100644
>--- a/source3/smbd/globals.h
>+++ b/source3/smbd/globals.h
>@@ -633,6 +633,7 @@ struct user_struct {
> };
> 
> struct smbd_server_connection {
>+	NTSTATUS status;
> 	int sock;
> 	const struct tsocket_address *local_address;
> 	const struct tsocket_address *remote_address;
>diff --git a/source3/smbd/process.c b/source3/smbd/process.c
>index 706d701..53d309d 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -152,6 +152,13 @@ bool srv_send_smb(struct smbd_server_connection *sconn, char *buffer,
> 	ssize_t ret;
> 	char *buf_out = buffer;
> 
>+	if (!NT_STATUS_IS_OK(sconn->status)) {
>+		/*
>+		 * we're not supposed to do any io
>+		 */
>+		return true;
>+	}
>+
> 	smbd_lock_socket(sconn);
> 
> 	if (do_signing) {
>@@ -2442,6 +2449,15 @@ static void smbd_server_connection_handler(struct tevent_context *ev,
> 	struct smbd_server_connection *conn = talloc_get_type(private_data,
> 					      struct smbd_server_connection);
> 
>+	if (!NT_STATUS_IS_OK(conn->status)) {
>+		/*
>+		 * we're not supposed to do any io
>+		 */
>+		TEVENT_FD_NOT_READABLE(conn->smb1.fde);
>+		TEVENT_FD_NOT_WRITEABLE(conn->smb1.fde);
>+		return;
>+	}
>+
> 	if (flags & TEVENT_FD_WRITE) {
> 		smbd_server_connection_write_handler(conn);
> 		return;
>@@ -2460,6 +2476,15 @@ static void smbd_server_echo_handler(struct tevent_context *ev,
> 	struct smbd_server_connection *conn = talloc_get_type(private_data,
> 					      struct smbd_server_connection);
> 
>+	if (!NT_STATUS_IS_OK(conn->status)) {
>+		/*
>+		 * we're not supposed to do any io
>+		 */
>+		TEVENT_FD_NOT_READABLE(conn->smb1.echo_handler.trusted_fde);
>+		TEVENT_FD_NOT_WRITEABLE(conn->smb1.echo_handler.trusted_fde);
>+		return;
>+	}
>+
> 	if (flags & TEVENT_FD_WRITE) {
> 		smbd_server_connection_write_handler(conn);
> 		return;
>diff --git a/source3/smbd/server_exit.c b/source3/smbd/server_exit.c
>index f99b9a4..3bfa2bc 100644
>--- a/source3/smbd/server_exit.c
>+++ b/source3/smbd/server_exit.c
>@@ -102,6 +102,17 @@ static void exit_server_common(enum server_exit_reason how,
> 	if (sconn) {
> 		NTSTATUS status;
> 
>+		if (NT_STATUS_IS_OK(sconn->status)) {
>+			switch (how) {
>+			case SERVER_EXIT_ABNORMAL:
>+				sconn->status = NT_STATUS_INTERNAL_ERROR;
>+				break;
>+			case SERVER_EXIT_NORMAL:
>+				sconn->status = NT_STATUS_LOCAL_DISCONNECT;
>+				break;
>+			}
>+		}
>+
> 		TALLOC_FREE(sconn->smb1.negprot.auth_context);
> 
> 		if (lp_log_writeable_files_on_exit()) {
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index e5f7de3..58eddee 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -2841,6 +2841,13 @@ static NTSTATUS smbd_smb2_request_next_incoming(struct smbd_server_connection *s
> 	size_t max_send_queue_len;
> 	size_t cur_send_queue_len;
> 
>+	if (!NT_STATUS_IS_OK(sconn->status)) {
>+		/*
>+		 * we're not supposed to do any io
>+		 */
>+		return NT_STATUS_OK;
>+	}
>+
> 	if (state->req != NULL) {
> 		/*
> 		 * if there is already a tstream_readv_pdu
>@@ -3085,6 +3092,15 @@ static NTSTATUS smbd_smb2_io_handler(struct smbd_server_connection *sconn,
> 	NTSTATUS status;
> 	NTTIME now;
> 
>+	if (!NT_STATUS_IS_OK(sconn->status)) {
>+		/*
>+		 * we're not supposed to do any io
>+		 */
>+		TEVENT_FD_NOT_READABLE(sconn->smb2.fde);
>+		TEVENT_FD_NOT_WRITEABLE(sconn->smb2.fde);
>+		return NT_STATUS_OK;
>+	}
>+
> 	if (fde_flags & TEVENT_FD_WRITE) {
> 		status = smbd_smb2_flush_send_queue(sconn);
> 		if (!NT_STATUS_IS_OK(status)) {
>-- 
>1.7.9.5
>
>
>From 380e83d02870c2670f250498d9c700a78b054ec0 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 17 Feb 2014 11:57:52 +0100
>Subject: [PATCH 3/4] s3:lib/ctdbd_conn: let release_ip_handler return bool
>
>If it returns true the passed ip address matched and we
>let a nested ctdb operation fail with NT_STATUS_ADDRESS_CLOSED.
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit 9677fae6aab26d2bf0884dc31516d2dcd8840c03)
>---
> source3/include/ctdbd_conn.h |    2 +-
> source3/lib/ctdbd_conn.c     |   36 ++++++++++++++++++++++++++++++------
> source3/smbd/process.c       |    6 ++++--
> 3 files changed, 35 insertions(+), 9 deletions(-)
>
>diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h
>index 64cb1d5..bba63f8 100644
>--- a/source3/include/ctdbd_conn.h
>+++ b/source3/include/ctdbd_conn.h
>@@ -76,7 +76,7 @@ NTSTATUS ctdbd_traverse(uint32_t db_id,
> NTSTATUS ctdbd_register_ips(struct ctdbd_connection *conn,
> 			    const struct sockaddr_storage *server,
> 			    const struct sockaddr_storage *client,
>-			    void (*release_ip_handler)(const char *ip_addr,
>+			    bool (*release_ip_handler)(const char *ip_addr,
> 						       void *private_data),
> 			    void *private_data);
> 
>diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
>index 630b22e..1c70b3b 100644
>--- a/source3/lib/ctdbd_conn.c
>+++ b/source3/lib/ctdbd_conn.c
>@@ -59,7 +59,7 @@ struct ctdbd_connection {
> 	struct ctdb_packet_context *pkt;
> 	struct tevent_fd *fde;
> 
>-	void (*release_ip_handler)(const char *ip_addr, void *private_data);
>+	bool (*release_ip_handler)(const char *ip_addr, void *private_data);
> 	void *release_ip_priv;
> };
> 
>@@ -427,10 +427,23 @@ static NTSTATUS ctdb_read_req(struct ctdbd_connection *conn, uint32_t reqid,
> 
> 		if ((conn->release_ip_handler != NULL)
> 		    && (msg->srvid == CTDB_SRVID_RELEASE_IP)) {
>+			bool ret;
>+
> 			/* must be dispatched immediately */
> 			DEBUG(10, ("received CTDB_SRVID_RELEASE_IP\n"));
>-			conn->release_ip_handler((const char *)msg->data,
>-						 conn->release_ip_priv);
>+			ret = conn->release_ip_handler((const char *)msg->data,
>+						       conn->release_ip_priv);
>+			if (ret) {
>+				/*
>+				 * We need to release the ip,
>+				 * so return an error to the upper layers.
>+				 *
>+				 * We make sure we don't trigger this again.
>+				 */
>+				conn->release_ip_handler = NULL;
>+				conn->release_ip_priv = NULL;
>+				return NT_STATUS_ADDRESS_CLOSED;
>+			}
> 			TALLOC_FREE(hdr);
> 			goto next_pkt;
> 		}
>@@ -629,10 +642,21 @@ static NTSTATUS ctdb_handle_message(uint8_t *buf, size_t length,
> 
> 	if ((conn->release_ip_handler != NULL)
> 	    && (msg->srvid == CTDB_SRVID_RELEASE_IP)) {
>+		bool ret;
>+
> 		/* must be dispatched immediately */
> 		DEBUG(10, ("received CTDB_SRVID_RELEASE_IP\n"));
>-		conn->release_ip_handler((const char *)msg->data,
>-					 conn->release_ip_priv);
>+		ret = conn->release_ip_handler((const char *)msg->data,
>+					       conn->release_ip_priv);
>+		if (ret) {
>+			/*
>+			 * We need to release the ip.
>+			 *
>+			 * We make sure we don't trigger this again.
>+			 */
>+			conn->release_ip_handler = NULL;
>+			conn->release_ip_priv = NULL;
>+		}
> 		TALLOC_FREE(buf);
> 		return NT_STATUS_OK;
> 	}
>@@ -1691,7 +1715,7 @@ static void smbd_ctdb_canonicalize_ip(const struct sockaddr_storage *in,
> NTSTATUS ctdbd_register_ips(struct ctdbd_connection *conn,
> 			    const struct sockaddr_storage *_server,
> 			    const struct sockaddr_storage *_client,
>-			    void (*release_ip_handler)(const char *ip_addr,
>+			    bool (*release_ip_handler)(const char *ip_addr,
> 						       void *private_data),
> 			    void *private_data)
> {
>diff --git a/source3/smbd/process.c b/source3/smbd/process.c
>index 53d309d..af4c9ed 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -2506,7 +2506,7 @@ struct smbd_release_ip_state {
> /****************************************************************************
> received when we should release a specific IP
> ****************************************************************************/
>-static void release_ip(const char *ip, void *priv)
>+static bool release_ip(const char *ip, void *priv)
> {
> 	struct smbd_release_ip_state *state =
> 		talloc_get_type_abort(priv,
>@@ -2543,8 +2543,10 @@ static void release_ip(const char *ip, void *priv)
> 		 */
> 		smbd_server_connection_terminate(state->sconn,
> 						 "CTDB_SRVID_RELEASE_IP");
>-		return;
>+		return true;
> 	}
>+
>+	return false;
> }
> 
> static NTSTATUS smbd_register_ips(struct smbd_server_connection *sconn,
>-- 
>1.7.9.5
>
>
>From 43af3e099a8c0e3aa828075068136112660b6947 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 13 Feb 2014 15:36:27 +0100
>Subject: [PATCH 4/4] s3:smbd: avoid invalid lock_order panic triggered by
> "CTDB_SRVID_RELEASE_IP"
>
>If smbd_server_connection_terminate("CTDB_SRVID_RELEASE_IP") is triggered from
>within ctdbd_migrate(), we got a smb_panic complaining about invalid
>lock_order, as ctdbd_migrate is called from dbwrap_fetch_locked().
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10444
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>
>Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
>Autobuild-Date(master): Fri Feb 21 14:51:51 CET 2014 on sn-devel-104
>(cherry picked from commit 33f10d06baf44e31d558bc5bd926c886915322cc)
>---
> source3/smbd/process.c |   41 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 39 insertions(+), 2 deletions(-)
>
>diff --git a/source3/smbd/process.c b/source3/smbd/process.c
>index af4c9ed..48dbfa1 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -2500,9 +2500,28 @@ static void smbd_server_echo_handler(struct tevent_context *ev,
> 
> struct smbd_release_ip_state {
> 	struct smbd_server_connection *sconn;
>+	struct tevent_immediate *im;
> 	char addr[INET6_ADDRSTRLEN];
> };
> 
>+static void smbd_release_ip_immediate(struct tevent_context *ctx,
>+				      struct tevent_immediate *im,
>+				      void *private_data)
>+{
>+	struct smbd_release_ip_state *state =
>+		talloc_get_type_abort(private_data,
>+		struct smbd_release_ip_state);
>+
>+	if (!NT_STATUS_EQUAL(state->sconn->status, NT_STATUS_ADDRESS_CLOSED)) {
>+		/*
>+		 * smbd_server_connection_terminate() already triggered ?
>+		 */
>+		return;
>+	}
>+
>+	smbd_server_connection_terminate(state->sconn, "CTDB_SRVID_RELEASE_IP");
>+}
>+
> /****************************************************************************
> received when we should release a specific IP
> ****************************************************************************/
>@@ -2514,6 +2533,11 @@ static bool release_ip(const char *ip, void *priv)
> 	const char *addr = state->addr;
> 	const char *p = addr;
> 
>+	if (!NT_STATUS_IS_OK(state->sconn->status)) {
>+		/* avoid recursion */
>+		return false;
>+	}
>+
> 	if (strncmp("::ffff:", addr, 7) == 0) {
> 		p = addr + 7;
> 	}
>@@ -2540,9 +2564,18 @@ static bool release_ip(const char *ip, void *priv)
> 		 * triggered and has implication on our process model,
> 		 * we can just use smbd_server_connection_terminate()
> 		 * (also for SMB1).
>+		 *
>+		 * We don't call smbd_server_connection_terminate() directly
>+		 * as we might be called from within ctdbd_migrate(),
>+		 * we need to defer our action to the next event loop
> 		 */
>-		smbd_server_connection_terminate(state->sconn,
>-						 "CTDB_SRVID_RELEASE_IP");
>+		tevent_schedule_immediate(state->im, state->sconn->ev_ctx,
>+					  smbd_release_ip_immediate, state);
>+
>+		/*
>+		 * Make sure we don't get any io on the connection.
>+		 */
>+		state->sconn->status = NT_STATUS_ADDRESS_CLOSED;
> 		return true;
> 	}
> 
>@@ -2566,6 +2599,10 @@ static NTSTATUS smbd_register_ips(struct smbd_server_connection *sconn,
> 		return NT_STATUS_NO_MEMORY;
> 	}
> 	state->sconn = sconn;
>+	state->im = tevent_create_immediate(state);
>+	if (state->im == NULL) {
>+		return NT_STATUS_NO_MEMORY;
>+	}
> 	if (print_sockaddr(state->addr, sizeof(state->addr), srv) == NULL) {
> 		return NT_STATUS_NO_MEMORY;
> 	}
>-- 
>1.7.9.5
>

Flags:

jra: review+
vl: review+

Actions: View

Attachments on bug 10444: 9705 | 9706 | 9707 | 9708