Fix a segfault when freeing SSL bufferevents in an unusual order

Have container bufferevents hold a reference to their underlying bufferevents.

(Commit message and minor revisions by nickm.)

diff --git a/bufferevent-internal.h b/bufferevent-internal.h
index a5ce7cb70d1b9cd5a1432a310227787f06bceb3c..a87a3c00b67e521afa311a9c579e7218df1d6267 100644 (file)
--- a/bufferevent-internal.h
+++ b/bufferevent-internal.h
@@ -217,6 +217,8 @@ void _bufferevent_incref_and_lock(struct bufferevent *bufev);
 /** Internal: Drop the reference count on bufev, freeing as necessary, and
  * unlocking it otherwise. */
 void _bufferevent_decref_and_unlock(struct bufferevent *bufev);
+/** Sometimes it is more clear to say "decref" than "free" */
+#define bufferevent_decref(b) bufferevent_free(b)
 
 /** Internal: If callbacks are deferred and we have a read callback, schedule
  * a readcb.  Otherwise just run the readcb. */

diff --git a/bufferevent.c b/bufferevent.c
index 012950b25fd6d7dc21ceb8e2b099024af2ce9e73..3f28eeeb34bd50a169782368560a050b9b0ae2df 100644 (file)
--- a/bufferevent.c
+++ b/bufferevent.c
@@ -512,12 +512,15 @@ _bufferevent_decref_and_unlock(struct bufferevent *bufev)
 {
        struct bufferevent_private *bufev_private =
            EVUTIL_UPCAST(bufev, struct bufferevent_private, bev);
+       struct bufferevent *underlying;
 
        if (--bufev_private->refcnt) {
                BEV_UNLOCK(bufev);
                return;
        }
 
+       underlying = bufferevent_get_underlying(bufev);
+
        /* Clean up the shared info */
        if (bufev->be_ops->destruct)
                bufev->be_ops->destruct(bufev);
@@ -536,6 +539,15 @@ _bufferevent_decref_and_unlock(struct bufferevent *bufev)
 
        /* Free the actual allocated memory. */
        mm_free(bufev - bufev->be_ops->mem_offset);
+
+       /* release the reference to underlying now that we no longer need
+        * the reference to it.  This is mainly in case our lock is shared
+        * with underlying.
+        * XXX Should we/can we just refcount evbuffer/bufferevent locks?
+        * It would probably save us some headaches.
+        */
+       if (underlying)
+               bufferevent_decref(underlying);
 }
 
 void

diff --git a/bufferevent_filter.c b/bufferevent_filter.c
index 7002b05ea6f913774a5efe3578da3ca9e87b261e..dedca445a47d9e622526295ca2e528b013b9f215 100644 (file)
--- a/bufferevent_filter.c
+++ b/bufferevent_filter.c
@@ -200,6 +200,7 @@ bufferevent_filter_new(struct bufferevent *underlying,
           bufferevent_filtered_outbuf_cb, bufev_f);
 
        _bufferevent_init_generic_timeout_cbs(downcast(bufev_f));
+       bufferevent_incref(underlying);
 
        return downcast(bufev_f);
 }

diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c
index c44fca30a99bd92b588b6c595c5e22fb7385f149..f121c5bedfd3d41b763be0971644113cc040f0ce 100644 (file)
--- a/bufferevent_openssl.c
+++ b/bufferevent_openssl.c
@@ -1101,8 +1101,10 @@ bufferevent_openssl_new_impl(struct event_base *base,
        if (options & BEV_OPT_THREADSAFE)
                bufferevent_enable_locking(&bev_ssl->bev.bev, NULL);
 
-       if (underlying)
+       if (underlying) {
                _bufferevent_init_generic_timeout_cbs(&bev_ssl->bev.bev);
+               bufferevent_incref(underlying);
+       }
 
        bev_ssl->state = state;
        bev_ssl->last_write = -1;