>From 7b6e4ba8a2878c0ff39b8aed2f86a1193a5ed7bc 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 --- 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 18a1a46..c656908 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 520b2f9e1edc407753cdf2dc7d69892e03628af9 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 --- 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 94111b6..bae3ed0 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 f5ca2f9..65d005d 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) { @@ -2445,6 +2452,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; @@ -2463,6 +2479,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 c656908..357c69a 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 fe42ac9..5e6bfd9 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 @@ -3087,6 +3094,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 f2e591d16747b56b06cb64f8f5f0199f1c10df04 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 --- 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 ce5c8ba..0f85229 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 6ab4bbe..4d90324 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; }; @@ -428,10 +428,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; } @@ -630,10 +643,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; } @@ -1692,7 +1716,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 65d005d..3dcc5d4 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -2509,7 +2509,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, @@ -2546,8 +2546,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 9b5a5d4143a21d9fa06b076db27dad20e5f51782 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 --- 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 3dcc5d4..41b3611 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -2503,9 +2503,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 ****************************************************************************/ @@ -2517,6 +2536,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; } @@ -2543,9 +2567,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; } @@ -2569,6 +2602,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