From 61e28a5c86b8dbe79693bcb681b8b3f58a0214a7 Mon Sep 17 00:00:00 2001 From: Soner Tari Date: Wed, 6 Apr 2022 16:44:46 +0300 Subject: [PATCH] Fix crash in split mode if we try to access srvdst while terminating Disable events and NULL callbacks of srvdst at assignment time to dst, not at termination. --- src/prototcp.c | 17 +++++++++++++++++ src/pxyconn.c | 15 ++++----------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/prototcp.c b/src/prototcp.c index cc07b14..76a478a 100644 --- a/src/prototcp.c +++ b/src/prototcp.c @@ -126,6 +126,23 @@ prototcp_setup_dst(pxy_conn_ctx_t *ctx) } else { // split mode ctx->dst = ctx->srvdst; + + // We reuse srvdst as dst or child dst, so srvdst == dst or child_dst. + // 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. + // 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. + bufferevent_setcb(ctx->srvdst.bev, NULL, NULL, NULL, NULL); + bufferevent_disable(ctx->srvdst.bev, EV_READ|EV_WRITE); + struct bufferevent *ubev = bufferevent_get_underlying(ctx->srvdst.bev); + if (ubev) { + bufferevent_setcb(ubev, NULL, NULL, NULL, NULL); + bufferevent_disable(ubev, EV_READ|EV_WRITE); + } + 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); } diff --git a/src/pxyconn.c b/src/pxyconn.c index 4872986..9784b1f 100644 --- a/src/pxyconn.c +++ b/src/pxyconn.c @@ -416,17 +416,9 @@ pxy_conn_free(pxy_conn_ctx_t *ctx, int by_requestor) // 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); - } else /*if (!ctx->divert || ctx->srvdst_xferred)*/ { - // We reuse srvdst as dst or child dst, so srvdst == dst or child_dst. - // 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. - // Note that we don't free anything here, but just disable callbacks and events. - // This does not seem to happen with srvdst_xferred, but just to be safe we do the same for it too. - // 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. - + } else if (ctx->srvdst_xferred) { + // @see prototcp_setup_dst() + // It does not seem to happen with srvdst_xferred, but just to be safe we do the same for it too. struct bufferevent *ubev = bufferevent_get_underlying(ctx->srvdst.bev); bufferevent_setcb(ctx->srvdst.bev, NULL, NULL, NULL, NULL); @@ -437,6 +429,7 @@ pxy_conn_free(pxy_conn_ctx_t *ctx, int by_requestor) bufferevent_disable(ubev, EV_READ|EV_WRITE); } } + // else if (!ctx->divert) {} ctx->srvdst.bev = NULL; }