From 26041a8ed86be8101573daf57c62c7806be7db08 Mon Sep 17 00:00:00 2001
From: Joachim Bauch <mail@joachim-bauch.de>
Date: Thu, 4 Aug 2011 23:39:15 +0200
Subject: [PATCH] prevent nested multicast references, reworked locking

---
 buffer.c                | 42 ++++++++++++-----------------------------
 evbuffer-internal.h     |  5 ++---
 include/event2/buffer.h |  6 +++++-
 test/regress_buffer.c   |  2 ++
 4 files changed, 21 insertions(+), 34 deletions(-)

diff --git a/buffer.c b/buffer.c
index 7c9d62c9..e2c7f2f8 100644
--- a/buffer.c
+++ b/buffer.c
@@ -221,10 +221,11 @@ evbuffer_chain_free(struct evbuffer_chain *chain)
 		    EVBUFFER_CHAIN_EXTRA(
 			    struct evbuffer_multicast_parent,
 			    chain);
+		EVUTIL_ASSERT(info->source != NULL);
 		EVUTIL_ASSERT(info->parent != NULL);
-		EVBUFFER_LOCK(info);
+		EVBUFFER_LOCK(info->source);
 		evbuffer_chain_decref(info->parent);
-		EVBUFFER_UNLOCK(info);
+		_evbuffer_decref_and_unlock(info->source);
 	}
 
 	mm_free(chain);
@@ -325,11 +326,8 @@ static inline void
 evbuffer_chain_decref(struct evbuffer_chain *chain)
 {
     EVUTIL_ASSERT(chain->refcnt > 0);
-    if (--chain->refcnt > 0) {
-        return;
-    }
-    
-    evbuffer_chain_free(chain);
+    --chain->refcnt;
+    // chain will be freed when parent buffer is released
 }
 
 struct evbuffer *
@@ -837,28 +835,12 @@ APPEND_CHAIN_MULTICAST(struct evbuffer *dst, struct evbuffer *src)
 			return;
 		}
 		extra = EVBUFFER_CHAIN_EXTRA(struct evbuffer_multicast_parent, tmp);
-		if (chain->flags & EVBUFFER_MULTICAST) {
-			// source chain is a reference itself
-			struct evbuffer_multicast_parent *info =
-				EVBUFFER_CHAIN_EXTRA(
-					struct evbuffer_multicast_parent,
-					chain);
-#ifndef _EVENT_DISABLE_THREAD_SUPPORT
-			extra->lock = info->lock;
-#endif
-			EVBUFFER_LOCK(info);
-			evbuffer_chain_incref(info->parent);
-			EVBUFFER_UNLOCK(info);
-			extra->parent = info->parent;
-		} else {
-			// reference source chain which now becomes immutable
-			evbuffer_chain_incref(chain);
-#ifndef _EVENT_DISABLE_THREAD_SUPPORT
-			extra->lock = src->lock;
-#endif
-			extra->parent = chain;
-			chain->flags |= EVBUFFER_IMMUTABLE;
-		}
+		// reference source chain which now becomes immutable
+		_evbuffer_incref(src);
+		extra->source = src;
+		evbuffer_chain_incref(chain);
+		extra->parent = chain;
+		chain->flags |= EVBUFFER_IMMUTABLE;
 		tmp->buffer_len = chain->buffer_len;
 		tmp->misalign = chain->misalign;
 		tmp->off = chain->off;
@@ -953,7 +935,7 @@ evbuffer_add_buffer_reference(struct evbuffer *outbuf, struct evbuffer *inbuf)
 	}
 
 	for (; chain; chain = chain->next) {
-		if ((chain->flags & (EVBUFFER_FILESEGMENT|EVBUFFER_SENDFILE)) != 0) {
+		if ((chain->flags & (EVBUFFER_FILESEGMENT|EVBUFFER_SENDFILE|EVBUFFER_MULTICAST)) != 0) {
 			// chain type can not be referenced
 			result = -1;
 			goto done;
diff --git a/evbuffer-internal.h b/evbuffer-internal.h
index 75d026d3..1d67973f 100644
--- a/evbuffer-internal.h
+++ b/evbuffer-internal.h
@@ -248,9 +248,8 @@ struct evbuffer_file_segment {
 /** Information about the multicast parent of a chain.  Lives at the
  * end of an evbuffer_chain with the EVBUFFER_MULTICAST flag set.  */
 struct evbuffer_multicast_parent {
-#ifndef _EVENT_DISABLE_THREAD_SUPPORT
-	void *lock; /**< lock prevent concurrent access to the parent */
-#endif
+	/** source buffer the multicast parent belongs to */
+	struct evbuffer *source;
 	/** multicast parent for this chain */
 	struct evbuffer_chain *parent;
 };
diff --git a/include/event2/buffer.h b/include/event2/buffer.h
index b8135f0d..951137fc 100644
--- a/include/event2/buffer.h
+++ b/include/event2/buffer.h
@@ -393,11 +393,15 @@ int evbuffer_add_buffer(struct evbuffer *outbuf, struct evbuffer *inbuf);
   This is a non-destructive add.  The data from one buffer is copied
   into the other buffer.  However, no unnecessary memory copies occur.
   
+  Note that buffers already containing buffer references can't be added
+  to other buffers.
+  
   @param outbuf the output buffer
   @param inbuf the input buffer
   @return 0 if successful, or -1 if an error occurred
  */
-int evbuffer_add_buffer_reference(struct evbuffer *outbuf, struct evbuffer *inbuf);
+int evbuffer_add_buffer_reference(struct evbuffer *outbuf,
+    struct evbuffer *inbuf);
 
 /**
    A cleanup function for a piece of memory added to an evbuffer by
diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index 0eda87cb..00240d92 100644
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -1544,7 +1544,9 @@ test_evbuffer_multicast(void *ptr)
 	tt_assert(buf2);
 
     tt_int_op(evbuffer_add_buffer_reference(buf2, buf1), ==, 0);
+    // nested references are not allowed
     tt_int_op(evbuffer_add_buffer_reference(buf2, buf2), ==, -1);
+    tt_int_op(evbuffer_add_buffer_reference(buf1, buf2), ==, -1);
 
     // both buffers contain the same amount of data
     tt_int_op(evbuffer_get_length(buf1), ==, evbuffer_get_length(buf1));