Fix crash bugs when a bufferevent's eventcb is not set.

In many places throughout the code, we called _bufferevent_run_eventcb
without checking whether the eventcb was actually set.  This would
work fine when the bufferevent's callbacks were deferred, but
otherwise the code would segfault.  Strangely, we always remembered to
check before calling the _bufferevent_run_{read,write}cb functions.

To prevent similar errors in the future, all of
_buferevent_run_{read,write,event}cb now check to make sure the
callback is actually set before invoking or deferring the callback.
This patch also removes the now-redundant checks for {read,write}cb.

diff --git a/bufferevent.c b/bufferevent.c
index 96b8ec7d7ad28b433d61ab31ad0073b70da0ff3c..c753a8676c1322446657d369247ec94174e68df8 100644 (file)
--- a/bufferevent.c
+++ b/bufferevent.c
@@ -175,6 +175,8 @@ _bufferevent_run_readcb(struct bufferevent *bufev)
        /* Requires that we hold the lock and a reference */
        struct bufferevent_private *p =
            EVUTIL_UPCAST(bufev, struct bufferevent_private, bev);
+       if (bufev->readcb == NULL)
+               return;
        if (p->options & BEV_OPT_DEFER_CALLBACKS) {
                p->readcb_pending = 1;
                if (!p->deferred.queued) {
@@ -192,6 +194,8 @@ _bufferevent_run_writecb(struct bufferevent *bufev)
        /* Requires that we hold the lock and a reference */
        struct bufferevent_private *p =
            EVUTIL_UPCAST(bufev, struct bufferevent_private, bev);
+       if (bufev->writecb == NULL)
+               return;
        if (p->options & BEV_OPT_DEFER_CALLBACKS) {
                p->writecb_pending = 1;
                if (!p->deferred.queued) {
@@ -209,6 +213,8 @@ _bufferevent_run_eventcb(struct bufferevent *bufev, short what)
        /* Requires that we hold the lock and a reference */
        struct bufferevent_private *p =
            EVUTIL_UPCAST(bufev, struct bufferevent_private, bev);
+       if (bufev->errorcb == NULL)
+               return;
        if (p->options & BEV_OPT_DEFER_CALLBACKS) {
                p->eventcb_pending |= what;
                p->errno_pending = EVUTIL_SOCKET_ERROR();

diff --git a/bufferevent_async.c b/bufferevent_async.c
index a8e92b70ee60932339a686f5129d74b465766b00..253a972bf97ccc6fa4c4a46ec657dbf4aacc4bf3 100644 (file)
--- a/bufferevent_async.c
+++ b/bufferevent_async.c
@@ -304,8 +304,7 @@ read_complete(struct event_overlapped *eo, uintptr_t key,
 
        if (ok && nbytes) {
                BEV_RESET_GENERIC_READ_TIMEOUT(bev);
-               if (bev->readcb != NULL &&
-                   evbuffer_get_length(bev->input) >= bev->wm_read.low)
+               if (evbuffer_get_length(bev->input) >= bev->wm_read.low)
                        _bufferevent_run_readcb(bev);
                bev_async_consider_reading(bev_a);
        } else if (!ok) {
@@ -337,8 +336,7 @@ write_complete(struct event_overlapped *eo, uintptr_t key,
 
        if (ok && nbytes) {
                BEV_RESET_GENERIC_WRITE_TIMEOUT(bev);
-               if (bev->writecb != NULL && 
-                   evbuffer_get_length(bev->output) <= bev->wm_write.low)
+               if (evbuffer_get_length(bev->output) <= bev->wm_write.low)
                        _bufferevent_run_writecb(bev);
                bev_async_consider_writing(bev_a);
        } else if (!ok) {

diff --git a/bufferevent_filter.c b/bufferevent_filter.c
index dedca445a47d9e622526295ca2e528b013b9f215..b7e1bc6a9da5258bc0e2cf7b03dd4ceb855fbf52 100644 (file)
--- a/bufferevent_filter.c
+++ b/bufferevent_filter.c
@@ -329,7 +329,7 @@ be_filter_process_output(struct bufferevent_filtered *bevf,
                         /* Or if we have filled the underlying output buffer. */
                         !be_underlying_writebuf_full(bevf,state));
 
-                if (processed && bufev->writecb &&
+                if (processed &&
                     evbuffer_get_length(bufev->output) <= bufev->wm_write.low) {
                         /* call the write callback.*/
                         _bufferevent_run_writecb(bufev);
@@ -394,8 +394,7 @@ be_filter_readcb(struct bufferevent *underlying, void *_me)
         * other places that can call process-input, and they should
         * force readcb calls as needed. */
        if (processed_any &&
-           evbuffer_get_length(bufev->input) >= bufev->wm_read.low &&
-           bufev->readcb != NULL)
+           evbuffer_get_length(bufev->input) >= bufev->wm_read.low)
                _bufferevent_run_readcb(bufev);
 
        _bufferevent_decref_and_unlock(bufev);
@@ -424,8 +423,7 @@ be_filter_eventcb(struct bufferevent *underlying, short what, void *_me)
 
        _bufferevent_incref_and_lock(bev);
        /* All we can really to is tell our own eventcb. */
-       if (bev->errorcb)
-               _bufferevent_run_eventcb(bev, what);
+       _bufferevent_run_eventcb(bev, what);
        _bufferevent_decref_and_unlock(bev);
 }
 

diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c
index f121c5bedfd3d41b763be0971644113cc040f0ce..26a701c0a7a82da50ff39df06085d774ab71b082 100644 (file)
--- a/bufferevent_openssl.c
+++ b/bufferevent_openssl.c
@@ -567,8 +567,7 @@ do_read(struct bufferevent_openssl *bev_ssl, int n_to_read)
                if (bev_ssl->underlying)
                        BEV_RESET_GENERIC_READ_TIMEOUT(bev);
 
-               if (evbuffer_get_length(input) >= bev->wm_read.low &&
-                   bev->readcb)
+               if (evbuffer_get_length(input) >= bev->wm_read.low)
                        _bufferevent_run_readcb(bev);
        }
 
@@ -631,8 +630,7 @@ do_write(struct bufferevent_openssl *bev_ssl, int atmost)
                if (bev_ssl->underlying)
                        BEV_RESET_GENERIC_WRITE_TIMEOUT(bev);
 
-               if (bev->writecb &&
-                   evbuffer_get_length(output) <= bev->wm_write.low)
+               if (evbuffer_get_length(output) <= bev->wm_write.low)
                        _bufferevent_run_writecb(bev);
        }
        return blocked ? 0 : 1;

diff --git a/bufferevent_pair.c b/bufferevent_pair.c
index 631827598a665be469806cebdac34368fc0b9f8d..34880462610880d7c11dcf05af52f8ffe5258ec5 100644 (file)
--- a/bufferevent_pair.c
+++ b/bufferevent_pair.c
@@ -177,10 +177,10 @@ be_pair_transfer(struct bufferevent *src, struct bufferevent *dst,
        src_size = evbuffer_get_length(src->output);
        dst_size = evbuffer_get_length(dst->input);
 
-       if (dst_size >= dst->wm_read.low && dst->readcb) {
+       if (dst_size >= dst->wm_read.low) {
                _bufferevent_run_readcb(dst);
        }
-       if (src_size <= src->wm_write.low && src->writecb) {
+       if (src_size <= src->wm_write.low) {
                _bufferevent_run_writecb(src);
        }
 done:
@@ -284,8 +284,7 @@ be_pair_flush(struct bufferevent *bev, short iotype,
                be_pair_transfer(bev, partner, 1);
 
        if (mode == BEV_FINISHED) {
-               if (partner->errorcb)
-                       _bufferevent_run_eventcb(partner, iotype|BEV_EVENT_EOF);
+               _bufferevent_run_eventcb(partner, iotype|BEV_EVENT_EOF);
        }
        decref_and_unlock(bev);
        return 0;

diff --git a/bufferevent_sock.c b/bufferevent_sock.c
index 61a369f6db6a635a92de3848ee59a6738b1bb24b..617c9110b51946fbca1600330502ef1c23308a55 100644 (file)
--- a/bufferevent_sock.c
+++ b/bufferevent_sock.c
@@ -165,8 +165,7 @@ bufferevent_readcb(evutil_socket_t fd, short event, void *arg)
 
 
        /* Invoke the user callback - must always be called last */
-       if (evbuffer_get_length(input) >= bufev->wm_read.low &&
-            bufev->readcb != NULL)
+       if (evbuffer_get_length(input) >= bufev->wm_read.low)
                _bufferevent_run_readcb(bufev);
 
        goto done;
@@ -259,7 +258,7 @@ bufferevent_writecb(evutil_socket_t fd, short event, void *arg)
         * Invoke the user callback if our buffer is drained or below the
         * low watermark.
         */
-       if (bufev->writecb != NULL && (res || !connected) &&
+       if ((res || !connected) &&
            evbuffer_get_length(bufev->output) <= bufev->wm_write.low)
                _bufferevent_run_writecb(bufev);