From d5f8b8bcb9066d0af64881201234b2ceb1e91357 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 28 Sep 2022 13:47:13 +0200 Subject: [PATCH 1/3] s4:messaging: add imessaging_init_discard_incoming() We often create imessaging contexts just for sending messages, but we'll never process incoming messages because a temporary event context was used and we just queue a lot of imessaging_post_state structures with immediate events. With imessaging_init_discard_incoming() we'll discard any incoming messages unless we have pending irpc requests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15201 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit a120fb1c724dfaed5a99e34aaf979502586f17c0) --- source4/lib/messaging/messaging.c | 72 +++++++++++++++++++++- source4/lib/messaging/messaging.h | 5 ++ source4/lib/messaging/messaging_internal.h | 9 +++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c index a00c35be0d58..99107c801769 100644 --- a/source4/lib/messaging/messaging.c +++ b/source4/lib/messaging/messaging.c @@ -429,6 +429,12 @@ static NTSTATUS imessaging_reinit(struct imessaging_context *msg) TALLOC_FREE(msg->msg_dgm_ref); + if (msg->discard_incoming) { + msg->num_incoming_listeners = 0; + } else { + msg->num_incoming_listeners = 1; + } + msg->server_id.pid = getpid(); msg->msg_dgm_ref = messaging_dgm_ref(msg, @@ -469,7 +475,9 @@ NTSTATUS imessaging_reinit_all(void) /* create the listening socket and setup the dispatcher */ -struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, +static struct imessaging_context *imessaging_init_internal( + TALLOC_CTX *mem_ctx, + bool discard_incoming, struct loadparm_context *lp_ctx, struct server_id server_id, struct tevent_context *ev) @@ -490,6 +498,12 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, return NULL; } msg->ev = ev; + msg->discard_incoming = discard_incoming; + if (msg->discard_incoming) { + msg->num_incoming_listeners = 0; + } else { + msg->num_incoming_listeners = 1; + } talloc_set_destructor(msg, imessaging_context_destructor); @@ -601,6 +615,36 @@ fail: return NULL; } +/* + create the listening socket and setup the dispatcher +*/ +struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, + struct loadparm_context *lp_ctx, + struct server_id server_id, + struct tevent_context *ev) +{ + bool discard_incoming = false; + return imessaging_init_internal(mem_ctx, + discard_incoming, + lp_ctx, + server_id, + ev); +} + +struct imessaging_context *imessaging_init_discard_incoming( + TALLOC_CTX *mem_ctx, + struct loadparm_context *lp_ctx, + struct server_id server_id, + struct tevent_context *ev) +{ + bool discard_incoming = true; + return imessaging_init_internal(mem_ctx, + discard_incoming, + lp_ctx, + server_id, + ev); +} + struct imessaging_post_state { struct imessaging_context *msg_ctx; struct imessaging_post_state **busy_ref; @@ -697,6 +741,22 @@ static void imessaging_dgm_recv(struct tevent_context *ev, return; } + if (msg->num_incoming_listeners == 0) { + struct server_id_buf selfbuf; + + message_hdr_get(&msg_type, &src, &dst, buf); + + DBG_DEBUG("not listening - discarding message from " + "src[%s] to dst[%s] (self[%s]) type=0x%x " + "on %s event context\n", + server_id_str_buf(src, &srcbuf), + server_id_str_buf(dst, &dstbuf), + server_id_str_buf(msg->server_id, &selfbuf), + (unsigned)msg_type, + (ev != msg->ev) ? "different" : "main"); + return; + } + if (ev != msg->ev) { int ret; ret = imessaging_post_self(msg, buf, buf_len); @@ -760,6 +820,7 @@ struct imessaging_context *imessaging_client_init(TALLOC_CTX *mem_ctx, return imessaging_init(mem_ctx, lp_ctx, id, ev); } + /* a list of registered irpc server functions */ @@ -975,6 +1036,12 @@ static int irpc_destructor(struct irpc_request *irpc) { if (irpc->callid != -1) { idr_remove(irpc->msg_ctx->idr, irpc->callid); + if (irpc->msg_ctx->discard_incoming) { + SMB_ASSERT(irpc->msg_ctx->num_incoming_listeners > 0); + } else { + SMB_ASSERT(irpc->msg_ctx->num_incoming_listeners > 1); + } + irpc->msg_ctx->num_incoming_listeners -= 1; irpc->callid = -1; } @@ -1168,6 +1235,9 @@ static struct tevent_req *irpc_bh_raw_call_send(TALLOC_CTX *mem_ctx, state->irpc->incoming.handler = irpc_bh_raw_call_incoming_handler; state->irpc->incoming.private_data = req; + /* make sure we accept incoming messages */ + SMB_ASSERT(state->irpc->msg_ctx->num_incoming_listeners < UINT64_MAX); + state->irpc->msg_ctx->num_incoming_listeners += 1; talloc_set_destructor(state->irpc, irpc_destructor); /* setup the header */ diff --git a/source4/lib/messaging/messaging.h b/source4/lib/messaging/messaging.h index 3fd788d1e420..e7ae9e8cc463 100644 --- a/source4/lib/messaging/messaging.h +++ b/source4/lib/messaging/messaging.h @@ -49,6 +49,11 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, struct loadparm_context *lp_ctx, struct server_id server_id, struct tevent_context *ev); +struct imessaging_context *imessaging_init_discard_incoming( + TALLOC_CTX *mem_ctx, + struct loadparm_context *lp_ctx, + struct server_id server_id, + struct tevent_context *ev); void imessaging_dgm_unref_ev(struct tevent_context *ev); NTSTATUS imessaging_reinit_all(void); int imessaging_cleanup(struct imessaging_context *msg); diff --git a/source4/lib/messaging/messaging_internal.h b/source4/lib/messaging/messaging_internal.h index 5e99734ad60e..ac254c226316 100644 --- a/source4/lib/messaging/messaging_internal.h +++ b/source4/lib/messaging/messaging_internal.h @@ -33,6 +33,15 @@ struct imessaging_context { struct server_id_db *names; struct timeval start_time; void *msg_dgm_ref; + /* + * The number of instances waiting for incoming + * messages. By default it's always greater than 0. + * + * If it's 0 we'll discard incoming messages, + * see imessaging_init_discard_imcoming(). + */ + bool discard_incoming; + uint64_t num_incoming_listeners; }; NTSTATUS imessaging_register_extra_handlers(struct imessaging_context *msg); -- 2.34.1 From df60063eb5d4256b4fddaeb936c7b86d689d0940 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 28 Sep 2022 14:14:41 +0200 Subject: [PATCH 2/3] s3:auth_samba4: make use of imessaging_init_discard_incoming() Otherwise we'll generate a memory leak of imessaging_post_state/ tevent_immediate structures per incoming message! BUG: https://bugzilla.samba.org/show_bug.cgi?id=15201 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit 32df5e4961cf064b72bb496157cc6092126d9b8e) --- source3/auth/auth_samba4.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source3/auth/auth_samba4.c b/source3/auth/auth_samba4.c index ff8dc94d2960..6c017ef4aa30 100644 --- a/source3/auth/auth_samba4.c +++ b/source3/auth/auth_samba4.c @@ -241,12 +241,12 @@ static NTSTATUS prepare_gensec(const struct auth_context *auth_context, return NT_STATUS_INVALID_SERVER_STATE; } - msg_ctx = imessaging_init(frame, + msg_ctx = imessaging_init_discard_incoming(frame, lp_ctx, *server_id, event_ctx); if (msg_ctx == NULL) { - DEBUG(1, ("imessaging_init failed\n")); + DEBUG(1, ("imessaging_init_discard_incoming failed\n")); TALLOC_FREE(frame); return NT_STATUS_INVALID_SERVER_STATE; } @@ -324,12 +324,12 @@ static NTSTATUS make_auth4_context_s4(const struct auth_context *auth_context, return NT_STATUS_INVALID_SERVER_STATE; } - msg_ctx = imessaging_init(frame, + msg_ctx = imessaging_init_discard_incoming(frame, lp_ctx, *server_id, event_ctx); if (msg_ctx == NULL) { - DEBUG(1, ("imessaging_init failed\n")); + DEBUG(1, ("imessaging_init_discard_incoming failed\n")); TALLOC_FREE(frame); return NT_STATUS_INVALID_SERVER_STATE; } -- 2.34.1 From 7bbc12d17b831efd2900a76c2cf7dab69b683d88 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 28 Sep 2022 14:27:09 +0200 Subject: [PATCH 3/3] s4:messaging: let imessaging_client_init() use imessaging_init_discard_incoming() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit imessaging_client_init() is for temporary stuff only, so we should drop (unexpected) incoming messages unless we expect irpc responses. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15201 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Thu Oct 13 13:32:30 UTC 2022 on sn-devel-184 (cherry picked from commit 266bcedc18efc52e29efde6bad220623a5423e30) --- source4/lib/messaging/messaging.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c index 99107c801769..8603c167ad4b 100644 --- a/source4/lib/messaging/messaging.c +++ b/source4/lib/messaging/messaging.c @@ -818,7 +818,7 @@ struct imessaging_context *imessaging_client_init(TALLOC_CTX *mem_ctx, /* This is because we are not in the s3 serverid database */ id.unique_id = SERVERID_UNIQUE_ID_NOT_TO_VERIFY; - return imessaging_init(mem_ctx, lp_ctx, id, ev); + return imessaging_init_discard_incoming(mem_ctx, lp_ctx, id, ev); } /* -- 2.34.1