From: Stefan Metzmacher <metze@samba.org>
Date: Mon, 9 Jul 2018 10:33:34 +0000 (+0200)
Subject: s3:messages: make the loop in msg_dgm_ref_recv() more robust against stale pointers
X-Git-Tag: tevent-0.9.37~59
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1a9d6ce58939678f88b3081fb91c3309ff3cddb7;p=thirdparty%2Fsamba.git

s3:messages: make the loop in msg_dgm_ref_recv() more robust against stale pointers

The interaction between msg_dgm_ref_recv() and msg_dgm_ref_destructor()
doesn't allow two references from messaging_dgm_ref() to be free'd
during the loop in msg_dgm_ref_recv().

In addition to the global 'refs' list, we also need to
have a global 'next_ref' pointer, which can be adjusted in
msg_dgm_ref_destructor().

As AD DC we hit this when using irpc in auth_winbind,
which uses imessaging_client_init().
In addition to the main messaging_dgm_ref() in smbd,
source3/auth/auth_samba4.c: prepare_gensec() and
make_auth4_context_s4() also generate a temporary
imessaging_context for auth_context->msg_ctx from within
auth_generic_prepare().

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13514

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
---

diff --git a/selftest/knownfail.d/imessaging b/selftest/knownfail.d/imessaging
deleted file mode 100644
index af8fa8d615a..00000000000
--- a/selftest/knownfail.d/imessaging
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.local.messaging.multi_ctx
diff --git a/source3/lib/messages_dgm_ref.c b/source3/lib/messages_dgm_ref.c
index 39d22700740..470dfbeabc7 100644
--- a/source3/lib/messages_dgm_ref.c
+++ b/source3/lib/messages_dgm_ref.c
@@ -35,6 +35,7 @@ struct msg_dgm_ref {
 
 static pid_t dgm_pid = 0;
 static struct msg_dgm_ref *refs = NULL;
+static struct msg_dgm_ref *next_ref = NULL;
 
 static int msg_dgm_ref_destructor(struct msg_dgm_ref *r);
 static void msg_dgm_ref_recv(struct tevent_context *ev,
@@ -121,16 +122,16 @@ static void msg_dgm_ref_recv(struct tevent_context *ev,
 			     const uint8_t *msg, size_t msg_len,
 			     int *fds, size_t num_fds, void *private_data)
 {
-	struct msg_dgm_ref *r, *next;
+	struct msg_dgm_ref *r;
 
 	/*
 	 * We have to broadcast incoming messages to all refs. The first ref
 	 * that grabs the fd's will get them.
 	 */
-	for (r = refs; r != NULL; r = next) {
+	for (r = refs; r != NULL; r = next_ref) {
 		bool active;
 
-		next = r->next;
+		next_ref = r->next;
 
 		active = messaging_dgm_fde_active(r->fde);
 		if (!active) {
@@ -150,6 +151,11 @@ static int msg_dgm_ref_destructor(struct msg_dgm_ref *r)
 	if (refs == NULL) {
 		abort();
 	}
+
+	if (r == next_ref) {
+		next_ref = r->next;
+	}
+
 	DLIST_REMOVE(refs, r);
 
 	TALLOC_FREE(r->fde);