Fixed potential double-readcb execution with openssl bufferevents.

the function do_read() will call SSL_read(), and if successful, will
call _bufferevent_run_readcb() before returning to consider_reading().

consider_reading() will then check SSL_pending() to make sure all
pending data has also been read. If it does not, do_read() is called
again.

The issue with this is the possibility that the function that is
executed by _bufferevent_run_readcb() called
bufferevent_disable(ssl_bev, EV_READ) before the second do_read(); In
this case, the users read callback is executed a second time. This is
potentially bad for applications that expect the bufferevent to stay
disabled until further notice. (this is why running openssl bufferevents
without DEFER_CALLBACKS has always been troublesome).
This commit is contained in:
Mark Ellzey 2012-03-30 15:08:40 -04:00 committed by Nick Mathewson
parent 30b6f889f7
commit 4e62cd167b

View File

@ -615,9 +615,6 @@ do_read(struct bufferevent_openssl *bev_ssl, int n_to_read)
evbuffer_commit_space(input, space, n_used);
if (bev_ssl->underlying)
BEV_RESET_GENERIC_READ_TIMEOUT(bev);
if (evbuffer_get_length(input) >= bev->wm_read.low)
_bufferevent_run_readcb(bev);
}
return blocked ? 0 : 1;
@ -757,6 +754,7 @@ consider_reading(struct bufferevent_openssl *bev_ssl)
{
int r;
int n_to_read;
int read_data = 0;
while (bev_ssl->write_blocked_on_read) {
r = do_write(bev_ssl, WRITE_FRAME);
@ -772,6 +770,8 @@ consider_reading(struct bufferevent_openssl *bev_ssl)
if (do_read(bev_ssl, n_to_read) <= 0)
break;
read_data = 1;
/* Read all pending data. This won't hit the network
* again, and will (most importantly) put us in a state
* where we don't need to read anything else until the
@ -800,6 +800,15 @@ consider_reading(struct bufferevent_openssl *bev_ssl)
n_to_read = bytes_to_read(bev_ssl);
}
if (read_data == 1) {
struct bufferevent *bev = &bev_ssl->bev.bev;
struct evbuffer *input = bev->input;
if (evbuffer_get_length(input) >= bev->wm_read.low) {
_bufferevent_run_readcb(bev);
}
}
if (!bev_ssl->underlying) {
/* Should be redundant, but let's avoid busy-looping */
if (bev_ssl->bev.read_suspended ||
@ -821,6 +830,14 @@ consider_writing(struct bufferevent_openssl *bev_ssl)
r = do_read(bev_ssl, 1024); /* XXXX 1024 is a hack */
if (r <= 0)
break;
else {
struct bufferevent *bev = &bev_ssl->bev.bev;
struct evbuffer *input = bev->input;
if (evbuffer_get_length(input) >= bev->wm_read.low) {
_bufferevent_run_readcb(bev);
}
}
}
if (bev_ssl->read_blocked_on_write)
return;