From: Nick Mathewson <nickm@torproject.org>
Date: Mon, 9 Aug 2010 16:08:40 +0000 (-0400)
Subject: Fix a nasty dangling-event bug when using rate-limiting groups
X-Git-Tag: release-2.0.7-rc~46
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0bffe43a15fbd43bc7826acb88a3bada528c8adf;p=libevent

Fix a nasty dangling-event bug when using rate-limiting groups

When we freed a bufferevent that was in a rate-limiting group and
blocked on IO, the process of freeing it caused it to get removed
from the group.  But removing the bufferevent from the group made
its limits get removed, which could make it get un-suspended and in
turn cause its events to get re-added.  Since we would then
immediately _free_ the events, this would result in dangling
pointers.

Fixes bug 3041007.
---

diff --git a/bufferevent.c b/bufferevent.c
index 4e8d9278..9080c5e2 100644
--- a/bufferevent.c
+++ b/bufferevent.c
@@ -608,7 +608,7 @@ _bufferevent_decref_and_unlock(struct bufferevent *bufev)
 
 	if (bufev_private->rate_limiting) {
 		if (bufev_private->rate_limiting->group)
-			bufferevent_remove_from_rate_limit_group(bufev);
+			bufferevent_remove_from_rate_limit_group_internal(bufev,0);
 		if (event_initialized(&bufev_private->rate_limiting->refill_bucket_event))
 			event_del(&bufev_private->rate_limiting->refill_bucket_event);
 		event_debug_unassign(&bufev_private->rate_limiting->refill_bucket_event);
diff --git a/bufferevent_ratelim.c b/bufferevent_ratelim.c
index cb8a3bf5..7ac579ac 100644
--- a/bufferevent_ratelim.c
+++ b/bufferevent_ratelim.c
@@ -715,6 +715,13 @@ bufferevent_add_to_rate_limit_group(struct bufferevent *bev,
 
 int
 bufferevent_remove_from_rate_limit_group(struct bufferevent *bev)
+{
+	return bufferevent_remove_from_rate_limit_group_internal(bev, 1);
+}
+
+int
+bufferevent_remove_from_rate_limit_group_internal(struct bufferevent *bev,
+    int unsuspend)
 {
 	struct bufferevent_private *bevp =
 	    EVUTIL_UPCAST(bev, struct bufferevent_private, bev);
@@ -728,8 +735,10 @@ bufferevent_remove_from_rate_limit_group(struct bufferevent *bev)
 		TAILQ_REMOVE(&g->members, bevp, rate_limiting->next_in_group);
 		UNLOCK_GROUP(g);
 	}
-	bufferevent_unsuspend_read(bev, BEV_SUSPEND_BW_GROUP);
-	bufferevent_unsuspend_write(bev, BEV_SUSPEND_BW_GROUP);
+	if (unsuspend) {
+		bufferevent_unsuspend_read(bev, BEV_SUSPEND_BW_GROUP);
+		bufferevent_unsuspend_write(bev, BEV_SUSPEND_BW_GROUP);
+	}
 	BEV_UNLOCK(bev);
 	return 0;
 }
diff --git a/ratelim-internal.h b/ratelim-internal.h
index a47e2408..d60f6376 100644
--- a/ratelim-internal.h
+++ b/ratelim-internal.h
@@ -84,6 +84,9 @@ int ev_token_bucket_init(struct ev_token_bucket *bucket,
     ev_uint32_t current_tick,
     int reinitialize);
 
+int bufferevent_remove_from_rate_limit_group_internal(struct bufferevent *bev,
+    int unsuspend);
+
 /** Decrease the read limit of 'b' by 'n' bytes */
 #define ev_token_bucket_decrement_read(b,n)	\
 	do {					\
diff --git a/test/test-ratelim.c b/test/test-ratelim.c
index 55c42932..306f1254 100644
--- a/test/test-ratelim.c
+++ b/test/test-ratelim.c
@@ -187,7 +187,11 @@ test_ratelimiting(void)
 	sin.sin_addr.s_addr = htonl(0x7f000001); /* 127.0.0.1 */
 	sin.sin_port = 0; /* unspecified port */
 
+	if (0)
+		event_enable_debug_mode();
+
 	base = event_base_new();
+
 	listener = evconnlistener_new_bind(base, echo_listenercb, base,
 	    LEV_OPT_CLOSE_ON_FREE|LEV_OPT_REUSEABLE, -1,
 	    (struct sockaddr *)&sin, sizeof(sin));