Attachment 17571 Details for Bug 15201 – Patches for v4-16-test

Patches for v4-16-test

bfixes-tmp416.txt (text/plain), 9.52 KB, created by Stefan Metzmacher on 2022-10-18 07:46:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2022-10-18 07:46:40 UTC

Size: 9.52 KB

patch

obsolete

>From d5f8b8bcb9066d0af64881201234b2ceb1e91357 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>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 <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(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 <metze@samba.org>
>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 <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(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 <metze@samba.org>
>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 <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>
>Autobuild-User(master): Ralph BÃ¶hme <slow@samba.org>
>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
>

Flags:

slow: review+

Actions: View

Attachments on bug 15201: 17568 | 17569 | 17570 | 17571