From ef337f8b2d52ddb699e3511c3fecd2dd6dbdc207 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Thu, 22 Feb 2024 17:18:15 +0100 Subject: [PATCH] BUG/MAJOR: promex: fix crash on deleted server Promex applet is used to dump many metrics. Some of them are related to a server instance. The applet can be interrupted in the middle of a dump, for example waiting for output buffer space. In this case, its context is save to resume dump on the correct instance. A crash can occur if dump is interrupted during servers loop. If the server instance is deleted during two scheduling of the promex applet, its context will still referenced the deleted server on resume. To fix this, use server refcount to prevent its deletion during parsing. As is manipulated in a lot of places, use a static helper function to automatically drop previous server refcount and increment the new one. To complete this change, also implement a release callback to ensure server refcount is decremented in case of early applet abort. A corresponding fix was first introduced in 3.0 dev branch. However, due to massive differences, a new patch has been designed on top of the current 2.9 tree. For reference, here is the 3.0 reference. f81c00cd447e21b07ee355fd5c2fb2a4787ea4a0 BUG/MAJOR: promex: fix crash on deleted server This must be backported to all stable releases. --- addons/promex/service-prometheus.c | 42 ++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/addons/promex/service-prometheus.c b/addons/promex/service-prometheus.c index 6885d20..e9ad44e 100644 --- a/addons/promex/service-prometheus.c +++ b/addons/promex/service-prometheus.c @@ -412,6 +412,15 @@ enum promex_srv_state promex_srv_status(struct server *sv) return state; } +/* Store in safely by using refcount to prevent server deletion. */ +static void promex_set_ctx_sv(struct promex_ctx *ctx, struct server *sv) +{ + srv_drop(ctx->sv); + ctx->sv = sv; + if (ctx->sv) + srv_take(ctx->sv); +} + /* Convert a field to its string representation and write it in , followed * by a newline, if there is enough space. non-numeric value are converted in * "NaN" because Prometheus only support numerical values (but it is unexepceted @@ -1125,16 +1134,16 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx) &val, labels, &out, max)) goto full; next_sv: - ctx->sv = sv->next; + promex_set_ctx_sv(ctx, sv->next); } next_px: ctx->px = px->next; - ctx->sv = (ctx->px ? ctx->px->srv : NULL); + promex_set_ctx_sv(ctx, ctx->px ? ctx->px->srv : NULL); } ctx->flags |= PROMEX_FL_METRIC_HDR; ctx->px = proxies_list; - ctx->sv = (ctx->px ? ctx->px->srv : NULL); + promex_set_ctx_sv(ctx, ctx->px ? ctx->px->srv : NULL); } @@ -1229,7 +1238,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = NULL; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_INFO_METRIC); ctx->obj_state = 0; ctx->field_num = INF_NAME; @@ -1249,7 +1258,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = proxies_list; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~PROMEX_FL_INFO_METRIC; ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_FRONT_METRIC); ctx->obj_state = 0; @@ -1270,7 +1279,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = proxies_list; ctx->st = NULL; ctx->li = LIST_NEXT(&proxies_list->conf.listeners, struct listener *, by_fe); - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~PROMEX_FL_FRONT_METRIC; ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_LI_METRIC); ctx->obj_state = 0; @@ -1291,7 +1300,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = proxies_list; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~PROMEX_FL_LI_METRIC; ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_BACK_METRIC); ctx->obj_state = 0; @@ -1312,7 +1321,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = proxies_list; ctx->st = NULL; ctx->li = NULL; - ctx->sv = ctx->px ? ctx->px->srv : NULL; + promex_set_ctx_sv(ctx, ctx->px ? ctx->px->srv : NULL); ctx->flags &= ~PROMEX_FL_BACK_METRIC; ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_SRV_METRIC); ctx->obj_state = 0; @@ -1333,7 +1342,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = NULL; ctx->st = stktables_list; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~(PROMEX_FL_METRIC_HDR|PROMEX_FL_SRV_METRIC); ctx->flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_STICKTABLE_METRIC); ctx->field_num = STICKTABLE_SIZE; @@ -1353,7 +1362,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = NULL; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags &= ~(PROMEX_FL_METRIC_HDR|PROMEX_FL_STICKTABLE_METRIC); ctx->field_num = 0; appctx->st1 = PROMEX_DUMPER_DONE; @@ -1374,7 +1383,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct ctx->px = NULL; ctx->st = NULL; ctx->li = NULL; - ctx->sv = NULL; + promex_set_ctx_sv(ctx, NULL); ctx->flags = 0; ctx->field_num = 0; appctx->st1 = PROMEX_DUMPER_DONE; @@ -1529,6 +1538,16 @@ static int promex_appctx_init(struct appctx *appctx) return 0; } +/* Callback function that releases a promex applet. This happens when the + * connection with the agent is closed. */ +static void promex_appctx_release(struct appctx *appctx) +{ + struct promex_ctx *ctx = appctx->svcctx; + + if (appctx->st1 == PROMEX_DUMPER_SRV) + srv_drop(ctx->sv); +} + /* The main I/O handler for the promex applet. */ static void promex_appctx_handle_io(struct appctx *appctx) { @@ -1621,6 +1640,7 @@ struct applet promex_applet = { .name = "", /* used for logging */ .init = promex_appctx_init, .fct = promex_appctx_handle_io, + .release = promex_appctx_release, }; static enum act_parse_ret service_parse_prometheus_exporter(const char **args, int *cur_arg, struct proxy *px, -- 1.7.10.4