be_openssl: get rid off hackish "fd_is_set", to fix some corner cases

This patch is a cleanup and a bug fix, it drops ```fd_is_set``` flag, and
replace it with some checks to event_initialized(), and now we will not call
event_assign() on already added event, plus we will delete event when we really
have to (this patch fixes the case when server is down, IOW before this patch
we will not call event_del() because ```fd_is_set``` was reset to 0) and this
will fix some issues with retries in http layer for ssl.

Reported-in: #258
Fixes: regress ssl/bufferevent_socketpair_timeout
Fixes: regress ssl/bufferevent_socketpair_timeout_freed_fd
This commit is contained in:
Azat Khuzhin 2015-08-22 21:38:18 +03:00
parent af85ecfccc
commit 40b0379833

View File

@ -320,8 +320,6 @@ struct bufferevent_openssl {
unsigned write_blocked_on_read : 1; unsigned write_blocked_on_read : 1;
/* Treat TCP close before SSL close on SSL >= v3 as clean EOF. */ /* Treat TCP close before SSL close on SSL >= v3 as clean EOF. */
unsigned allow_dirty_shutdown : 1; unsigned allow_dirty_shutdown : 1;
/* XXXX */
unsigned fd_is_set : 1;
/* XXX */ /* XXX */
unsigned n_errors : 2; unsigned n_errors : 2;
@ -968,27 +966,27 @@ set_open_callbacks(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
} else { } else {
struct bufferevent *bev = &bev_ssl->bev.bev; struct bufferevent *bev = &bev_ssl->bev.bev;
int rpending=0, wpending=0, r1=0, r2=0; int rpending=0, wpending=0, r1=0, r2=0;
if (fd < 0 && bev_ssl->fd_is_set) int new_fd = fd < 0 ? event_get_fd(&bev->ev_read) : fd;
fd = event_get_fd(&bev->ev_read);
if (bev_ssl->fd_is_set) { if (fd >= 0 && event_initialized(&bev->ev_read)) {
rpending = event_pending(&bev->ev_read, EV_READ, NULL); rpending = event_pending(&bev->ev_read, EV_READ, NULL);
wpending = event_pending(&bev->ev_write, EV_WRITE, NULL); wpending = event_pending(&bev->ev_write, EV_WRITE, NULL);
event_del(&bev->ev_read); event_del(&bev->ev_read);
event_del(&bev->ev_write); event_del(&bev->ev_write);
} }
event_assign(&bev->ev_read, bev->ev_base, fd,
event_assign(&bev->ev_read, bev->ev_base, new_fd,
EV_READ|EV_PERSIST|EV_FINALIZE, EV_READ|EV_PERSIST|EV_FINALIZE,
be_openssl_readeventcb, bev_ssl); be_openssl_readeventcb, bev_ssl);
event_assign(&bev->ev_write, bev->ev_base, fd, event_assign(&bev->ev_write, bev->ev_base, new_fd,
EV_WRITE|EV_PERSIST|EV_FINALIZE, EV_WRITE|EV_PERSIST|EV_FINALIZE,
be_openssl_writeeventcb, bev_ssl); be_openssl_writeeventcb, bev_ssl);
if (rpending) if (rpending)
r1 = bufferevent_add_event_(&bev->ev_read, &bev->timeout_read); r1 = bufferevent_add_event_(&bev->ev_read, &bev->timeout_read);
if (wpending) if (wpending)
r2 = bufferevent_add_event_(&bev->ev_write, &bev->timeout_write); r2 = bufferevent_add_event_(&bev->ev_write, &bev->timeout_write);
if (fd >= 0) {
bev_ssl->fd_is_set = 1;
}
return (r1<0 || r2<0) ? -1 : 0; return (r1<0 || r2<0) ? -1 : 0;
} }
} }
@ -1011,9 +1009,10 @@ do_handshake(struct bufferevent_openssl *bev_ssl)
decrement_buckets(bev_ssl); decrement_buckets(bev_ssl);
if (r==1) { if (r==1) {
int fd = event_get_fd(&bev_ssl->bev.bev.ev_read);
/* We're done! */ /* We're done! */
bev_ssl->state = BUFFEREVENT_SSL_OPEN; bev_ssl->state = BUFFEREVENT_SSL_OPEN;
set_open_callbacks(bev_ssl, -1); /* XXXX handle failure */ set_open_callbacks(bev_ssl, fd); /* XXXX handle failure */
/* Call do_read and do_write as needed */ /* Call do_read and do_write as needed */
bufferevent_enable(&bev_ssl->bev.bev, bev_ssl->bev.bev.enabled); bufferevent_enable(&bev_ssl->bev.bev, bev_ssl->bev.bev.enabled);
bufferevent_run_eventcb_(&bev_ssl->bev.bev, bufferevent_run_eventcb_(&bev_ssl->bev.bev,
@ -1074,12 +1073,12 @@ set_handshake_callbacks(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
} else { } else {
struct bufferevent *bev = &bev_ssl->bev.bev; struct bufferevent *bev = &bev_ssl->bev.bev;
int r1=0, r2=0; int r1=0, r2=0;
if (fd < 0 && bev_ssl->fd_is_set)
fd = event_get_fd(&bev->ev_read); if (event_initialized(&bev->ev_read)) {
if (bev_ssl->fd_is_set) {
event_del(&bev->ev_read); event_del(&bev->ev_read);
event_del(&bev->ev_write); event_del(&bev->ev_write);
} }
event_assign(&bev->ev_read, bev->ev_base, fd, event_assign(&bev->ev_read, bev->ev_base, fd,
EV_READ|EV_PERSIST|EV_FINALIZE, EV_READ|EV_PERSIST|EV_FINALIZE,
be_openssl_handshakeeventcb, bev_ssl); be_openssl_handshakeeventcb, bev_ssl);
@ -1089,12 +1088,23 @@ set_handshake_callbacks(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
if (fd >= 0) { if (fd >= 0) {
r1 = bufferevent_add_event_(&bev->ev_read, &bev->timeout_read); r1 = bufferevent_add_event_(&bev->ev_read, &bev->timeout_read);
r2 = bufferevent_add_event_(&bev->ev_write, &bev->timeout_write); r2 = bufferevent_add_event_(&bev->ev_write, &bev->timeout_write);
bev_ssl->fd_is_set = 1;
} }
return (r1<0 || r2<0) ? -1 : 0; return (r1<0 || r2<0) ? -1 : 0;
} }
} }
static int
set_handshake_callbacks_auto(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
{
if (!bev_ssl->underlying) {
struct bufferevent *bev = &bev_ssl->bev.bev;
if (event_initialized(&bev->ev_read) && fd < 0) {
fd = event_get_fd(&bev->ev_read);
}
}
return set_handshake_callbacks(bev_ssl, fd);
}
int int
bufferevent_ssl_renegotiate(struct bufferevent *bev) bufferevent_ssl_renegotiate(struct bufferevent *bev)
{ {
@ -1104,7 +1114,7 @@ bufferevent_ssl_renegotiate(struct bufferevent *bev)
if (SSL_renegotiate(bev_ssl->ssl) < 0) if (SSL_renegotiate(bev_ssl->ssl) < 0)
return -1; return -1;
bev_ssl->state = BUFFEREVENT_SSL_CONNECTING; bev_ssl->state = BUFFEREVENT_SSL_CONNECTING;
if (set_handshake_callbacks(bev_ssl, -1) < 0) if (set_handshake_callbacks_auto(bev_ssl, -1) < 0)
return -1; return -1;
if (!bev_ssl->underlying) if (!bev_ssl->underlying)
return do_handshake(bev_ssl); return do_handshake(bev_ssl);
@ -1162,8 +1172,6 @@ static int
be_openssl_disable(struct bufferevent *bev, short events) be_openssl_disable(struct bufferevent *bev, short events)
{ {
struct bufferevent_openssl *bev_ssl = upcast(bev); struct bufferevent_openssl *bev_ssl = upcast(bev);
if (bev_ssl->state != BUFFEREVENT_SSL_OPEN)
return 0;
if (events & EV_READ) if (events & EV_READ)
stop_reading(bev_ssl); stop_reading(bev_ssl);
@ -1274,25 +1282,16 @@ be_openssl_ctrl(struct bufferevent *bev,
BIO *bio; BIO *bio;
bio = BIO_new_socket(data->fd, 0); bio = BIO_new_socket(data->fd, 0);
SSL_set_bio(bev_ssl->ssl, bio, bio); SSL_set_bio(bev_ssl->ssl, bio, bio);
bev_ssl->fd_is_set = 1;
} }
if (data->fd == -1)
bev_ssl->fd_is_set = 0;
if (bev_ssl->state == BUFFEREVENT_SSL_OPEN) if (bev_ssl->state == BUFFEREVENT_SSL_OPEN)
return set_open_callbacks(bev_ssl, data->fd); return set_open_callbacks(bev_ssl, data->fd);
else { else {
return set_handshake_callbacks(bev_ssl, data->fd); return set_handshake_callbacks(bev_ssl, data->fd);
} }
case BEV_CTRL_GET_FD: case BEV_CTRL_GET_FD:
if (bev_ssl->underlying)
return -1;
if (!bev_ssl->fd_is_set)
return -1;
data->fd = event_get_fd(&bev->ev_read); data->fd = event_get_fd(&bev->ev_read);
return 0; return 0;
case BEV_CTRL_GET_UNDERLYING: case BEV_CTRL_GET_UNDERLYING:
if (!bev_ssl->underlying)
return -1;
data->ptr = bev_ssl->underlying; data->ptr = bev_ssl->underlying;
return 0; return 0;
case BEV_CTRL_CANCEL_ALL: case BEV_CTRL_CANCEL_ALL:
@ -1360,12 +1359,12 @@ bufferevent_openssl_new_impl(struct event_base *base,
switch (state) { switch (state) {
case BUFFEREVENT_SSL_ACCEPTING: case BUFFEREVENT_SSL_ACCEPTING:
SSL_set_accept_state(bev_ssl->ssl); SSL_set_accept_state(bev_ssl->ssl);
if (set_handshake_callbacks(bev_ssl, fd) < 0) if (set_handshake_callbacks_auto(bev_ssl, fd) < 0)
goto err; goto err;
break; break;
case BUFFEREVENT_SSL_CONNECTING: case BUFFEREVENT_SSL_CONNECTING:
SSL_set_connect_state(bev_ssl->ssl); SSL_set_connect_state(bev_ssl->ssl);
if (set_handshake_callbacks(bev_ssl, fd) < 0) if (set_handshake_callbacks_auto(bev_ssl, fd) < 0)
goto err; goto err;
break; break;
case BUFFEREVENT_SSL_OPEN: case BUFFEREVENT_SSL_OPEN:
@ -1383,14 +1382,14 @@ bufferevent_openssl_new_impl(struct event_base *base,
bufferevent_suspend_read_(underlying, bufferevent_suspend_read_(underlying,
BEV_SUSPEND_FILT_READ); BEV_SUSPEND_FILT_READ);
} else { } else {
bev_ssl->bev.bev.enabled = EV_READ|EV_WRITE; struct bufferevent *bev = &bev_ssl->bev.bev;
if (bev_ssl->fd_is_set) { bev->enabled = EV_READ|EV_WRITE;
if (state != BUFFEREVENT_SSL_OPEN) if (state != BUFFEREVENT_SSL_OPEN)
if (event_add(&bev_ssl->bev.bev.ev_read, NULL) < 0) if (event_add(&bev->ev_read, NULL) < 0)
goto err; goto err;
if (event_add(&bev_ssl->bev.bev.ev_write, NULL) < 0) if (event_initialized(&bev->ev_write))
if (event_add(&bev->ev_write, NULL) < 0)
goto err; goto err;
}
} }
return &bev_ssl->bev.bev; return &bev_ssl->bev.bev;