From e1daeec874ab7ce982a2af7b1d5f0ab6aaf8e996 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 17 Feb 2014 12:05:42 +0100 Subject: [PATCH 1/4] s3:smbd: simplify exit_server_common() Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (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 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 Reviewed-by: Volker Lendecke (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 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 Reviewed-by: Volker Lendecke (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 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 Reviewed-by: Volker Lendecke Autobuild-User(master): Stefan Metzmacher 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