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 4e8d92780e495121af41bcdf0acc5944fc06aa77..9080c5e21a614a5107717ced1eb1cc369cff7cbf 100644 (file)
--- 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 cb8a3bf5262561780f3c39f004dca2bfd2720df7..7ac579ac26808a33d3198fa0693ab00d3b78f460 100644 (file)
--- 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 a47e2408ee9cc0f218a4b9d2a1fc72049e85e7bc..d60f6376dd265ac0830f901fdef8569fca01a286 100644 (file)
--- 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 55c42932966d6b1d53811f6a9e4ee0c1307f15aa..306f12541ca975c3b763073320ab75e35a8d1f87 100644 (file)
--- 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));