smbd: Remove callback for release_ip when "state" is free'ed
authorVolker Lendecke <vl@samba.org>
Thu, 12 Oct 2023 15:19:45 +0000 (17:19 +0200)
committerJule Anger <janger@samba.org>
Sat, 16 Dec 2023 14:29:10 +0000 (14:29 +0000)
If a client connects to a non-public address first followed by a connect
to public address with the same client_guid and a connection to
the non-public address gets disconnected first, we hit by a use-after-free
talloc_get_type_abort() called from release_ip() as
"xconn" is already gone, taking smbd_release_ip_state with it.

We need to decide between calling ctdbd_unregister_ips() by default, as
it means the tcp connection is really gone and ctdb needs to remove the
'tickle' information.  But when a connection was passed to a different
smbd process, we need to use ctdbd_passed_ips() as the tcp connection is
still alive and the 'tickle' information should not be removed within
ctdb.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15523

Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>

Signed-off-by: Volker Lendecke <vl@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Martin Schwenke <martin@meltin.net>
(cherry picked from commit ddf47e7fe314e0f5bf71ff53e35350e0ba530d08)

selftest/flapping.d/smbXsrv_client_ctdb_registered_ips [deleted file]
selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips [deleted file]
source3/smbd/smb2_negprot.c
source3/smbd/smb2_process.c

diff --git a/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips b/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips
deleted file mode 100644 (file)
index 740bb87..0000000
+++ /dev/null
@@ -1,4 +0,0 @@
-^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step5:.ctdb_gettickles.NUM
-^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step5:.ctdb_gettickles.DST
-^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step6:.ctdb_gettickles.NUM
-^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step6:.ctdb_gettickles.DST
diff --git a/selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips b/selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips
deleted file mode 100644 (file)
index 8bce2bb..0000000
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step7:.smbstatus.0.sessions
index c0e558b2c4d00442bcda8b8f8b7f4c7fcec5b439..d89e6d49d2feb916fb7da5d0e7fe033be06c9025 100644 (file)
@@ -877,7 +877,14 @@ static void smbd_smb2_request_process_negprot_mc_done(struct tevent_req *subreq)
        if (NT_STATUS_EQUAL(status, NT_STATUS_MESSAGE_RETRIEVED)) {
                /*
                 * The connection was passed to another process
+                *
+                * We mark the error as NT_STATUS_CONNECTION_IN_USE,
+                * in order to indicate to low level code if
+                * ctdbd_unregister_ips() or ctdbd_passed_ips()
+                * is more useful.
                 */
+               smbXsrv_connection_disconnect_transport(xconn,
+                                               NT_STATUS_CONNECTION_IN_USE);
                smbd_server_connection_terminate(xconn,
                                                 "passed connection");
                /*
index 11f556c88acef88416e24b35270f436299a452a8..7afc845293905bd90ffd42cc127ea8120aa306a2 100644 (file)
@@ -1017,9 +1017,37 @@ static void smbd_server_connection_handler(struct tevent_context *ev,
 struct smbd_release_ip_state {
        struct smbXsrv_connection *xconn;
        struct tevent_immediate *im;
+       struct sockaddr_storage srv;
+       struct sockaddr_storage clnt;
        char addr[INET6_ADDRSTRLEN];
 };
 
+static int release_ip(struct tevent_context *ev,
+                     uint32_t src_vnn,
+                     uint32_t dst_vnn,
+                     uint64_t dst_srvid,
+                     const uint8_t *msg,
+                     size_t msglen,
+                     void *private_data);
+
+static int smbd_release_ip_state_destructor(struct smbd_release_ip_state *s)
+{
+       struct ctdbd_connection *cconn = messaging_ctdb_connection();
+       struct smbXsrv_connection *xconn = s->xconn;
+
+       if (cconn == NULL) {
+               return 0;
+       }
+
+       if (NT_STATUS_EQUAL(xconn->transport.status, NT_STATUS_CONNECTION_IN_USE)) {
+               ctdbd_passed_ips(cconn, &s->srv, &s->clnt, release_ip, s);
+       } else {
+               ctdbd_unregister_ips(cconn, &s->srv, &s->clnt, release_ip, s);
+       }
+
+       return 0;
+}
+
 static void smbd_release_ip_immediate(struct tevent_context *ctx,
                                      struct tevent_immediate *im,
                                      void *private_data)
@@ -1162,6 +1190,8 @@ static NTSTATUS smbd_register_ips(struct smbXsrv_connection *xconn,
        if (state->im == NULL) {
                return NT_STATUS_NO_MEMORY;
        }
+       state->srv = *srv;
+       state->clnt = *clnt;
        if (print_sockaddr(state->addr, sizeof(state->addr), srv) == NULL) {
                return NT_STATUS_NO_MEMORY;
        }
@@ -1185,6 +1215,9 @@ static NTSTATUS smbd_register_ips(struct smbXsrv_connection *xconn,
        if (ret != 0) {
                return map_nt_error_from_unix(ret);
        }
+
+       talloc_set_destructor(state, smbd_release_ip_state_destructor);
+
        return NT_STATUS_OK;
 }