From 2640bae75e3abab1a9959b2de14dad29020852de Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 12 Oct 2023 17:19:45 +0200 Subject: [PATCH] smbd: Remove callback for release_ip when "state" is free'ed 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 Signed-off-by: Volker Lendecke Signed-off-by: Stefan Metzmacher Reviewed-by: Martin Schwenke (cherry picked from commit ddf47e7fe314e0f5bf71ff53e35350e0ba530d08) --- .../flapping.d/smbXsrv_client_ctdb_registered_ips | 4 --- .../knownfail.d/smbXsrv_client_ctdb_registered_ips | 1 - source3/smbd/smb2_negprot.c | 7 +++++ source3/smbd/smb2_process.c | 33 ++++++++++++++++++++++ 4 files changed, 40 insertions(+), 5 deletions(-) delete mode 100644 selftest/flapping.d/smbXsrv_client_ctdb_registered_ips delete mode 100644 selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips diff --git a/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips b/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips deleted file mode 100644 index 740bb87251b..00000000000 --- a/selftest/flapping.d/smbXsrv_client_ctdb_registered_ips +++ /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 index 8bce2bbb5db..00000000000 --- a/selftest/knownfail.d/smbXsrv_client_ctdb_registered_ips +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.smbXsrv_client_ctdb_registered_ips.step7:.smbstatus.0.sessions diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c index 885769be24d..fbed543f563 100644 --- a/source3/smbd/smb2_negprot.c +++ b/source3/smbd/smb2_negprot.c @@ -880,7 +880,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"); /* diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c index d55f80646ac..fbbe4ef3992 100644 --- a/source3/smbd/smb2_process.c +++ b/source3/smbd/smb2_process.c @@ -984,9 +984,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) @@ -1129,6 +1157,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; } @@ -1152,6 +1182,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; } -- 2.11.4.GIT