From: Nick M. <ni...@us...> - 2009-12-30 01:05:11
|
Author: Nick Mathewson <ni...@to...> Date: Tue, 29 Dec 2009 19:50:03 -0500 Subject: Fix crash bugs when a bufferevent's eventcb is not set. Commit: 2e8eeea3e86b1f577e34ef2bc4f7099d848ae787 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. --- bufferevent.c | 6 ++++++ bufferevent_async.c | 6 ++---- bufferevent_filter.c | 8 +++----- bufferevent_openssl.c | 6 ++---- bufferevent_pair.c | 7 +++---- bufferevent_sock.c | 5 ++--- 6 files changed, 18 insertions(+), 20 deletions(-) diff --git a/bufferevent.c b/bufferevent.c index 96b8ec7..c753a86 100644 --- 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 a8e92b7..253a972 100644 --- 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 dedca44..b7e1bc6 100644 --- 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 f121c5b..26a701c 100644 --- 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 6318275..3488046 100644 --- 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 61a369f..617c911 100644 --- 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); -- 1.6.3 |