From a773df54cef1707984fe2c8b4b5866b35ae0c66d Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Fri, 18 Dec 2009 16:24:41 -0500 Subject: [PATCH] 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.) --- bufferevent-internal.h | 2 ++ bufferevent.c | 12 ++++++++++++ bufferevent_filter.c | 1 + bufferevent_openssl.c | 4 +++- 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/bufferevent-internal.h b/bufferevent-internal.h index a5ce7cb7..a87a3c00 100644 --- 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 012950b2..3f28eeeb 100644 --- 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 7002b05e..dedca445 100644 --- 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 c44fca30..f121c5be 100644 --- 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; -- 2.40.0