Remove srvdst_xferred flag

Setting srvdst.bev to NULL can be used as a flag to indicate that we
have reused the srvdst of the parent as the dst of the first child conn.
This also prevents access to srvdst.bev after we do the xfer, without
any extra flag.
This commit is contained in:
Soner Tari 2022-04-08 17:12:36 +03:00
parent 02a6cc12e6
commit 76ea48f2d0
7 changed files with 42 additions and 46 deletions

View File

@ -82,20 +82,19 @@ protopassthrough_engage(pxy_conn_ctx_t *ctx)
// In split mode, srvdst is used as dst, so it should be freed as dst below
// If srvdst has been xferred to the first child conn, the child should free it, not the parent
if (ctx->divert && !ctx->srvdst_xferred) {
if (ctx->srvdst.bev) {
ctx->srvdst.free(ctx->srvdst.bev, ctx);
ctx->srvdst.bev = NULL;
}
ctx->srvdst.bev = NULL;
ctx->srvdst.ssl = NULL;
ctx->connected = 0;
// Close and free dst if open
// Make sure bev is not NULL, as dst may not have been initialized yet
if (!ctx->dst.closed && ctx->dst.bev) {
ctx->dst.closed = 1;
if (ctx->dst.bev) {
ctx->dst.free(ctx->dst.bev, ctx);
ctx->dst.bev = NULL;
ctx->dst_fd = 0;
ctx->dst.closed = 1;
}
// Free any/all data of the previous proto

View File

@ -1528,17 +1528,20 @@ protossl_setup_dst_ssl_child(pxy_conn_child_ctx_t *ctx)
int
protossl_setup_dst_child(pxy_conn_child_ctx_t *ctx)
{
if (!ctx->conn->srvdst_xferred) {
if (ctx->conn->srvdst.bev) {
// Reuse srvdst of parent in the first child conn
ctx->dst = ctx->conn->srvdst;
ctx->conn->srvdst_xferred = 1;
// See the comments in prototcp_setup_dst()
prototcp_disable_events_srvdst(ctx->conn);
prototcp_disable_srvdst(ctx->conn);
bufferevent_setcb(ctx->dst.bev, pxy_bev_readcb_child, pxy_bev_writecb_child, pxy_bev_eventcb_child, ctx);
ctx->protoctx->bev_eventcb(ctx->dst.bev, BEV_EVENT_CONNECTED, ctx);
} else {
// Return 1 to signal the caller that we have reused srvdst as the dst of the first child conn
return 1;
}
else {
if (protossl_setup_dst_ssl_child(ctx) == -1) {
return -1;
}
@ -1556,13 +1559,13 @@ protossl_setup_dst_child(pxy_conn_child_ctx_t *ctx)
return 0;
}
void
int
protossl_connect_child(pxy_conn_child_ctx_t *ctx)
{
log_finest("ENTER");
/* create server-side socket and eventbuffer */
protossl_setup_dst_child(ctx);
return protossl_setup_dst_child(ctx);
}
static int NONNULL(1)

View File

@ -42,7 +42,7 @@ SSL *protossl_dstssl_create(pxy_conn_ctx_t *) NONNULL(1);
void protossl_free(pxy_conn_ctx_t *) NONNULL(1);
void protossl_init_conn(evutil_socket_t, short, void *);
int protossl_conn_connect(pxy_conn_ctx_t *) NONNULL(1) WUNRES;
void protossl_connect_child(pxy_conn_child_ctx_t *) NONNULL(1);
int protossl_connect_child(pxy_conn_child_ctx_t *) NONNULL(1) WUNRES;
int protossl_enable_src(pxy_conn_ctx_t *) NONNULL(1);
@ -53,7 +53,7 @@ int protossl_setup_src_new_bev_ssl_accepting(pxy_conn_ctx_t *) NONNULL(1);
int protossl_setup_dst_ssl(pxy_conn_ctx_t *) NONNULL(1);
int protossl_setup_dst_new_bev_ssl_connecting(pxy_conn_ctx_t *) NONNULL(1);
int protossl_setup_dst_ssl_child(pxy_conn_child_ctx_t *) NONNULL(1);
int protossl_setup_dst_new_bev_ssl_connecting_child(pxy_conn_child_ctx_t *) NONNULL(1);
int protossl_setup_dst_new_bev_ssl_connecting_child(pxy_conn_child_ctx_t *) NONNULL(1) WUNRES;
int protossl_setup_dst_child(pxy_conn_child_ctx_t *) NONNULL(1);
int protossl_setup_srvdst(pxy_conn_ctx_t *ctx) NONNULL(1);

View File

@ -112,7 +112,7 @@ prototcp_setup_src(pxy_conn_ctx_t *ctx)
}
void
prototcp_disable_events_srvdst(pxy_conn_ctx_t *ctx)
prototcp_disable_srvdst(pxy_conn_ctx_t *ctx)
{
bufferevent_setcb(ctx->srvdst.bev, NULL, NULL, NULL, NULL);
bufferevent_disable(ctx->srvdst.bev, EV_READ|EV_WRITE);
@ -122,6 +122,8 @@ prototcp_disable_events_srvdst(pxy_conn_ctx_t *ctx)
bufferevent_setcb(ubev, NULL, NULL, NULL, NULL);
bufferevent_disable(ubev, EV_READ|EV_WRITE);
}
// Do not access srvdst.bev from this point on
ctx->srvdst.bev = NULL;
}
int
@ -144,12 +146,12 @@ prototcp_setup_dst(pxy_conn_ctx_t *ctx)
// But if we don't NULL the callbacks of srvdst in split mode,
// we randomly but rarely get a second eof event for srvdst during conn termination (especially on arm64),
// which crashes us with signal 11 or 10, because the first eof event for dst frees the ctx.
// This does not seem to happen with srvdst_xferred, but just to be safe we do the same for it too.
// This does not seem to happen with srvdst xferred, but just to be safe we do the same for it too.
// Note that we don't free anything here, but just disable callbacks and events.
// This seems to be an issue with libevent.
// @todo Why does libevent raise the same event again for an already disabled and freed conn end?
// Note again that srvdst == dst or child_dst here.
prototcp_disable_events_srvdst(ctx);
prototcp_disable_srvdst(ctx);
bufferevent_setcb(ctx->dst.bev, pxy_bev_readcb, pxy_bev_writecb, pxy_bev_eventcb, ctx);
ctx->protoctx->bev_eventcb(ctx->dst.bev, BEV_EVENT_CONNECTED, ctx);
@ -203,17 +205,20 @@ prototcp_setup_src_child(pxy_conn_child_ctx_t *ctx)
int
prototcp_setup_dst_child(pxy_conn_child_ctx_t *ctx)
{
if (!ctx->conn->srvdst_xferred) {
if (ctx->conn->srvdst.bev) {
// Reuse srvdst of parent in the first child conn
ctx->dst = ctx->conn->srvdst;
ctx->conn->srvdst_xferred = 1;
// See the comments in prototcp_setup_dst()
prototcp_disable_events_srvdst(ctx->conn);
prototcp_disable_srvdst(ctx->conn);
bufferevent_setcb(ctx->dst.bev, pxy_bev_readcb_child, pxy_bev_writecb_child, pxy_bev_eventcb_child, ctx);
ctx->protoctx->bev_eventcb(ctx->dst.bev, BEV_EVENT_CONNECTED, ctx);
} else {
// Return 1 to signal the caller that we have reused srvdst as the dst of the first child conn
return 1;
}
else {
ctx->dst.ssl = NULL;
ctx->dst.bev = prototcp_bufferevent_setup_child(ctx, -1);
if (!ctx->dst.bev) {
@ -226,13 +231,13 @@ prototcp_setup_dst_child(pxy_conn_child_ctx_t *ctx)
return 0;
}
static void NONNULL(1)
static int NONNULL(1) WUNRES
prototcp_connect_child(pxy_conn_child_ctx_t *ctx)
{
log_finest("ENTER");
/* create server-side socket and eventbuffer */
prototcp_setup_dst_child(ctx);
return prototcp_setup_dst_child(ctx);
}
void

View File

@ -82,12 +82,12 @@ int prototcp_enable_src(pxy_conn_ctx_t *) NONNULL(1);
void prototcp_bev_eventcb_srvdst(struct bufferevent *, short, pxy_conn_ctx_t *) NONNULL(1);
int prototcp_setup_src(pxy_conn_ctx_t *) NONNULL(1);
void prototcp_disable_events_srvdst(pxy_conn_ctx_t *) NONNULL(1);
void prototcp_disable_srvdst(pxy_conn_ctx_t *) NONNULL(1);
int prototcp_setup_dst(pxy_conn_ctx_t *) NONNULL(1);
int prototcp_setup_srvdst(pxy_conn_ctx_t *) NONNULL(1);
int prototcp_setup_src_child(pxy_conn_child_ctx_t *) NONNULL(1);
int prototcp_setup_dst_child(pxy_conn_child_ctx_t *) NONNULL(1);
int prototcp_setup_dst_child(pxy_conn_child_ctx_t *) NONNULL(1) WUNRES;
protocol_t prototcp_setup(pxy_conn_ctx_t *) NONNULL(1);
protocol_t prototcp_setup_child(pxy_conn_child_ctx_t *) NONNULL(1);

View File

@ -256,22 +256,15 @@ pxy_conn_free_child(pxy_conn_child_ctx_t *ctx)
ctx->dst.bev = NULL;
}
// Save conn and srvdst_xferred before freeing ctx
// Save conn before freeing ctx
pxy_conn_ctx_t *conn = ctx->conn;
unsigned int srvdst_xferred = conn->srvdst_xferred;
pxy_conn_detach_child(ctx);
pxy_conn_ctx_free_child(ctx);
// If there is no child left, free child_evcl asap by calling pxy_conn_free_children()
if (!conn->children) {
if (!conn->children)
pxy_conn_free_children(conn);
}
// If this is the first child, NULL srvdst.bev, so we don't try to access it from this point on
if (srvdst_xferred) {
conn->srvdst.bev = NULL;
}
}
void
@ -414,9 +407,7 @@ pxy_conn_free(pxy_conn_ctx_t *ctx, int by_requestor)
if (ctx->srvdst.bev) {
// In split mode, srvdst is used as dst, so it should be freed as dst below
// If srvdst has been xferred to the first child conn, the child should free it, not the parent
if (ctx->divert && !ctx->srvdst_xferred) {
ctx->srvdst.free(ctx->srvdst.bev, ctx);
}
ctx->srvdst.free(ctx->srvdst.bev, ctx);
ctx->srvdst.bev = NULL;
}
@ -1084,12 +1075,12 @@ pxy_listener_acceptcb_child(UNUSED struct evconnlistener *listener, evutil_socke
// @attention fd (child_ctx->fd) is different from child event listener fd (ctx->child_fd)
ctx->thr->max_fd = max(ctx->thr->max_fd, child_ctx->fd);
ctx->child_src_fd = child_ctx->fd;
/* create server-side socket and eventbuffer */
// Children rely on the findings of parent
child_ctx->protoctx->connectcb(child_ctx);
int connect_retval = child_ctx->protoctx->connectcb(child_ctx);
if (ctx->term || ctx->enomem) {
if (connect_retval == -1 || ctx->term || ctx->enomem) {
goto out;
}
@ -1097,14 +1088,15 @@ pxy_listener_acceptcb_child(UNUSED struct evconnlistener *listener, evutil_socke
log_dbg_printf("Child connecting to [%s]:%s\n", STRORDASH(ctx->dsthost_str), STRORDASH(ctx->dstport_str));
}
/* initiate connection, except for the first child conn which uses the parent's srvdst as dst */
if (child_ctx->dst.bev != ctx->srvdst.bev) {
// initiate connection, except for the first child conn which uses the parent's srvdst as dst
// connectcb returns 1 if we have reused srvdst as the dst of the first child conn, and 0 for the other child conns
if (connect_retval == 0) {
if (bufferevent_socket_connect(child_ctx->dst.bev, (struct sockaddr *)&ctx->dstaddr, ctx->dstaddrlen) == -1) {
pxy_conn_term(ctx, 1);
goto out;
}
}
child_ctx->dst_fd = bufferevent_getfd(child_ctx->dst.bev);
ctx->child_dst_fd = child_ctx->dst_fd;
ctx->thr->max_fd = max(ctx->thr->max_fd, child_ctx->dst_fd);

View File

@ -81,7 +81,7 @@ typedef int (*proto_validate_func_t)(pxy_conn_ctx_t *, char *, size_t);
typedef void (*proto_classify_user_func_t)(pxy_conn_ctx_t *);
#endif /* !WITHOUT_USERAUTH */
typedef void (*child_connect_func_t)(pxy_conn_child_ctx_t *);
typedef int (*child_connect_func_t)(pxy_conn_child_ctx_t *);
typedef void (*child_proto_free_func_t)(pxy_conn_child_ctx_t *);
typedef void (*set_watermark_func_t)(struct bufferevent *, pxy_conn_ctx_t *, struct bufferevent *);
@ -255,9 +255,6 @@ struct pxy_conn_ctx {
unsigned int term : 1; /* 0 until term requested */
unsigned int term_requestor : 1; /* 1 client, 0 server side */
// srvdst_xferred flag is important not to access srvdst.bev
// after the first child is freed
unsigned int srvdst_xferred : 1; /* 1 if srvdst xferred to child */
struct pxy_conn_desc srvdst;
struct event *ev;