[SCM] Samba Shared Repository - branch v4-10-test updated

Karolin Seeger kseeger at samba.org
Mon Nov 25 14:57:04 UTC 2019


The branch, v4-10-test has been updated
       via  939a0c8bb24 ctdb-tcp: Close inflight connecting TCP sockets after fork
       via  47b1e70b6aa ctdb-tcp: Drop tracking of file descriptor for incoming connections
       via  39f93ff7121 ctdb-tcp: Avoid orphaning the TCP incoming queue
       via  5eb95d0d088 ctdb-tcp: Check incoming queue to see if incoming connection is up
       via  471835acb08 s3: libsmb: Ensure return from net_share_enum_rpc() sets cli->raw_status on error.
      from  0b6c23def7b s3: utils: smbtree. Ensure we don't call cli_RNetShareEnum() on an SMB1 connection.

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-10-test


- Log -----------------------------------------------------------------
commit 939a0c8bb24412ee728ce03c0104de19d5733cf5
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Nov 7 15:26:01 2019 +0100

    ctdb-tcp: Close inflight connecting TCP sockets after fork
    
    Commit c68b6f96f26 changed the talloc hierarchy such that outgoing TCP sockets
    while sitting in the async connect() syscall are not freed via
    ctdb_tcp_shutdown() anymore, they are hanging off a longer-running structure.
    Free this structure as well.
    
    If an outgoing TCP socket leaks into a long-running child process (possibly the
    recovery daemon), this connection will never be closed as seen by the
    destination node. Because with recent changes incoming connections will not be
    accepted as long as any incoming connection is alive, with that socket leak
    into the recovery daemon we will never again be able to successfully connect to
    the node that is affected by this leak. Further attempts to connect will be
    discarded by the destination as long as the recovery daemon keeps this socket
    alive.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14175
    RN: Avoid communication breakdown on node reconnect
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit a6d99d9e5c5bc58e6d56be7a6c1dbc7c8d1a882f)
    
    Autobuild-User(v4-10-test): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(v4-10-test): Mon Nov 25 14:56:53 UTC 2019 on sn-devel-144

commit 47b1e70b6aa737f723365e7196e36139f2ed6a50
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Oct 29 17:28:22 2019 +1100

    ctdb-tcp: Drop tracking of file descriptor for incoming connections
    
    This file descriptor is owned by the incoming queue.  It will be
    closed when the queue is torn down.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14175
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit bf47bc18bb8a94231870ef821c0352b7a15c2e28)

commit 39f93ff712146dcee412141f2b6583eb4ce092d6
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Oct 29 15:29:11 2019 +1100

    ctdb-tcp: Avoid orphaning the TCP incoming queue
    
    CTDB's incoming queue handling does not check whether an existing
    queue exists, so can overwrite the pointer to the queue.  This used to
    be harmless until commit c68b6f96f26664459187ab2fbd56767fb31767e0
    changed the read callback to use a parent structure as the callback
    data.  Instead of cleaning up an orphaned queue on disconnect, as
    before, this will now free the new queue.
    
    At first glance it doesn't seem possible that 2 incoming connections
    from the same node could be processed before the intervening
    disconnect.  However, the incoming connections and disconnect occur on
    different file descriptors.  The queue can become orphaned on node A
    when the following sequence occurs:
    
    1. Node A comes up
    2. Node A accepts an incoming connection from node B
    3. Node B processes a timeout before noticing that outgoing the queue is writable
    4. Node B tears down the outgoing connection to node A
    5. Node B initiates a new connection to node A
    6. Node A accepts an incoming connection from node B
    
    Node A processes then the disconnect of the old incoming connection
    from (2) but tears down the new incoming connection from (6).  This
    then occurs until the originally affected node is restarted.
    
    However, due to the number of outgoing connection attempts and
    associated teardowns, this induces the same behaviour on the
    corresponding incoming queue on all nodes that node A attempts to
    connect to.  Therefore, other nodes become affected and need to be
    restarted too.
    
    As a result, the whole cluster probably needs to be restarted to
    recover from this situation.
    
    The problem can occur any time CTDB is started on a node.
    
    The fix is to avoid accepting new incoming connections when a queue
    for incoming connections is already present.  The connecting node will
    simply retry establishing its outgoing connection.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14175
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit d0baad257e511280ff3e5c7372c38c43df841070)

commit 5eb95d0d0888d381a871e828d0e9de82a785aa09
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Oct 29 15:25:26 2019 +1100

    ctdb-tcp: Check incoming queue to see if incoming connection is up
    
    This makes it consistent with the reverse case.  Also, in_fd will soon
    be removed.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14175
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit e62b3a05a874db13a848573d2e2fb1c157393b9c)

commit 471835acb088ad2e8ea106fea358584b05eb3fe6
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Oct 31 14:38:35 2019 -0700

    s3: libsmb: Ensure return from net_share_enum_rpc() sets cli->raw_status on error.
    
    Convert net_share_enum_rpc() to return an NTSTATUS and ensure the
    status is set correctly on error so SMBC_errno() can return it.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14176
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    
    Autobuild-User(master): Andreas Schneider <asn at cryptomilk.org>
    Autobuild-Date(master): Tue Nov  5 12:36:48 UTC 2019 on sn-devel-184
    
    (cherry picked from commit ff47cc661d432a9337ade9a232a4f49164652812)

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

Summary of changes:
 ctdb/tcp/ctdb_tcp.h         |  1 -
 ctdb/tcp/tcp_connect.c      | 11 ++++++++---
 ctdb/tcp/tcp_init.c         | 12 ++++++------
 ctdb/tcp/tcp_io.c           |  2 --
 source3/libsmb/libsmb_dir.c | 33 ++++++++++++++++++++++-----------
 5 files changed, 36 insertions(+), 23 deletions(-)


Changeset truncated at 500 lines:

diff --git a/ctdb/tcp/ctdb_tcp.h b/ctdb/tcp/ctdb_tcp.h
index 9a615fc6393..daabad74297 100644
--- a/ctdb/tcp/ctdb_tcp.h
+++ b/ctdb/tcp/ctdb_tcp.h
@@ -37,7 +37,6 @@ struct ctdb_tcp_node {
 	struct tevent_timer *connect_te;
 
 	struct ctdb_context *ctdb;
-	int in_fd;
 	struct ctdb_queue *in_queue;
 };
 
diff --git a/ctdb/tcp/tcp_connect.c b/ctdb/tcp/tcp_connect.c
index f02340c1789..0b5d021480a 100644
--- a/ctdb/tcp/tcp_connect.c
+++ b/ctdb/tcp/tcp_connect.c
@@ -153,7 +153,7 @@ static void ctdb_node_connect_write(struct tevent_context *ev,
 	 * as connected, but only if the corresponding listening
 	 * socket is also connected
 	 */
-	if (tnode->in_fd != -1) {
+	if (tnode->in_queue != NULL) {
 		node->ctdb->upcalls->node_connected(node);
 	}
 }
@@ -312,6 +312,13 @@ static void ctdb_listen_event(struct tevent_context *ev, struct tevent_fd *fde,
 		return;
 	}
 
+	if (tnode->in_queue != NULL) {
+		DBG_ERR("Incoming queue active, rejecting connection from %s\n",
+			ctdb_addr_to_str(&addr));
+		close(fd);
+		return;
+	}
+
 	ret = set_blocking(fd, false);
 	if (ret != 0) {
 		DBG_ERR("Failed to set socket non-blocking (%s)\n",
@@ -348,8 +355,6 @@ static void ctdb_listen_event(struct tevent_context *ev, struct tevent_fd *fde,
 		return;
 	}
 
-	tnode->in_fd = fd;
-
        /*
 	* Mark the connecting node as connected, but only if the
 	* corresponding outbound connected is also up
diff --git a/ctdb/tcp/tcp_init.c b/ctdb/tcp/tcp_init.c
index 91d4e992c8f..dc92abd4e6c 100644
--- a/ctdb/tcp/tcp_init.c
+++ b/ctdb/tcp/tcp_init.c
@@ -43,11 +43,6 @@ static int tnode_destructor(struct ctdb_tcp_node *tnode)
 		tnode->out_fd = -1;
 	}
 
-	if (tnode->in_fd != -1) {
-		close(tnode->in_fd);
-		tnode->in_fd = -1;
-	}
-
 	return 0;
 }
 
@@ -61,7 +56,6 @@ static int ctdb_tcp_add_node(struct ctdb_node *node)
 	CTDB_NO_MEMORY(node->ctdb, tnode);
 
 	tnode->out_fd = -1;
-	tnode->in_fd = -1;
 	tnode->ctdb = node->ctdb;
 
 	node->private_data = tnode;
@@ -143,8 +137,14 @@ static void ctdb_tcp_shutdown(struct ctdb_context *ctdb)
 {
 	struct ctdb_tcp *ctcp = talloc_get_type(ctdb->private_data,
 						struct ctdb_tcp);
+	uint32_t i;
+
 	talloc_free(ctcp);
 	ctdb->private_data = NULL;
+
+	for (i=0; i<ctdb->num_nodes; i++) {
+		TALLOC_FREE(ctdb->nodes[i]->private_data);
+	}
 }
 
 /*
diff --git a/ctdb/tcp/tcp_io.c b/ctdb/tcp/tcp_io.c
index e8ebff887e1..2d8ec0f7062 100644
--- a/ctdb/tcp/tcp_io.c
+++ b/ctdb/tcp/tcp_io.c
@@ -76,8 +76,6 @@ void ctdb_tcp_read_cb(uint8_t *data, size_t cnt, void *args)
 
 failed:
 	TALLOC_FREE(tnode->in_queue);
-	close(tnode->in_fd);
-	tnode->in_fd = -1;
 	node->ctdb->upcalls->node_dead(node);
 
 	TALLOC_FREE(data);
diff --git a/source3/libsmb/libsmb_dir.c b/source3/libsmb/libsmb_dir.c
index ab20a127c49..14bfcdec6a7 100644
--- a/source3/libsmb/libsmb_dir.c
+++ b/source3/libsmb/libsmb_dir.c
@@ -350,7 +350,7 @@ dir_list_fn(const char *mnt,
 	return NT_STATUS_OK;
 }
 
-static int
+static NTSTATUS
 net_share_enum_rpc(struct cli_state *cli,
                    void (*fn)(const char *name,
                               uint32_t type,
@@ -377,7 +377,7 @@ net_share_enum_rpc(struct cli_state *cli,
 					     &pipe_hnd);
         if (!NT_STATUS_IS_OK(nt_status)) {
                 DEBUG(1, ("net_share_enum_rpc pipe open fail!\n"));
-                return -1;
+		goto done;
         }
 
 	ZERO_STRUCT(info_ctr);
@@ -400,18 +400,18 @@ net_share_enum_rpc(struct cli_state *cli,
         /* Was it successful? */
 	if (!NT_STATUS_IS_OK(nt_status)) {
                 /*  Nope.  Go clean up. */
-		result = ntstatus_to_werror(nt_status);
 		goto done;
 	}
 
 	if (!W_ERROR_IS_OK(result)) {
                 /*  Nope.  Go clean up. */
+		nt_status = werror_to_ntstatus(result);
 		goto done;
         }
 
 	if (total_entries == 0) {
                 /*  Nope.  Go clean up. */
-		result = WERR_GEN_FAILURE;
+		nt_status = NT_STATUS_NOT_FOUND;
 		goto done;
 	}
 
@@ -436,7 +436,7 @@ done:
         TALLOC_FREE(pipe_hnd);
 
         /* Tell 'em if it worked */
-        return W_ERROR_IS_OK(result) ? 0 : -1;
+        return nt_status;
 }
 
 
@@ -825,7 +825,7 @@ SMBC_opendir_ctx(SMBCCTX *context,
 				}
 			} else if (srv ||
                                    (resolve_name(server, &rem_ss, 0x20, false))) {
-				int rc;
+				NTSTATUS status;
 
                                 /*
                                  * If we hadn't found the server, get one now
@@ -852,27 +852,38 @@ SMBC_opendir_ctx(SMBCCTX *context,
 
                                 /* List the shares ... */
 
-				rc = net_share_enum_rpc(srv->cli,
+				status = net_share_enum_rpc(srv->cli,
 							list_fn,
 							(void *)dir);
-				if (rc != 0 &&
+				if (!NT_STATUS_IS_OK(status) &&
 				    smbXcli_conn_protocol(srv->cli->conn) <=
 						PROTOCOL_NT1) {
 					/*
 					 * Only call cli_RNetShareEnum()
 					 * on SMB1 connections, not SMB2+.
 					 */
-					rc = cli_RNetShareEnum(srv->cli,
+					int rc = cli_RNetShareEnum(srv->cli,
 							       list_fn,
 							       (void *)dir);
+					if (rc != 0) {
+						status = cli_nt_error(srv->cli);
+					} else {
+						status = NT_STATUS_OK;
+					}
 				}
-				if (rc != 0) {
-					errno = cli_errno(srv->cli);
+				if (!NT_STATUS_IS_OK(status)) {
+					/*
+					 * Set cli->raw_status so SMBC_errno()
+					 * will correctly return the error.
+					 */
+					srv->cli->raw_status = status;
 					if (dir != NULL) {
 						SAFE_FREE(dir->fname);
 						SAFE_FREE(dir);
 					}
 					TALLOC_FREE(frame);
+					errno = map_errno_from_nt_status(
+								status);
 					return NULL;
 				}
                         } else {


-- 
Samba Shared Repository



More information about the samba-cvs mailing list