Skip to content

Commit b4f64c0

Browse files
committed
BUG/MEDIUM: promex: server iteration may rely on stale server
When performing a promex dump, even though we hold reference on server during resumption after a yield (ie: buffer full), the refcount mechanism only guarantees that the server pointer will be valid upon resumption, not that its content will be consistent. As such, sv->next may be garbage upon resumption. Instead, we must rely on the watcher mechanism to iterate over server list when resumption is involved like we already do for stats and lua handlers. It must be backported anywhere 071ae8c (" BUG/MEDIUM: stats/server: use watcher to track server during stats dump") was (up to 2.8 it seems)
1 parent d38b918 commit b4f64c0

1 file changed

Lines changed: 14 additions & 13 deletions

File tree

addons/promex/service-prometheus.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ struct promex_ctx {
8282
unsigned field_num; /* current field number (ST_I_PX_* etc) */
8383
unsigned mod_field_num; /* first field number of the current module (ST_I_PX_* etc) */
8484
int obj_state; /* current state among PROMEX_{FRONT|BACK|SRV|LI}_STATE_* */
85+
struct watcher srv_watch; /* watcher to automatically update next pointer */
8586
struct list modules; /* list of promex modules to export */
8687
struct eb_root filters; /* list of filters to apply on metrics name */
8788
};
@@ -1244,8 +1245,10 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
12441245
if ((px->flags & PR_FL_DISABLED) || px->uuid <= 0 || !(px->cap & PR_CAP_BE))
12451246
goto next_px;
12461247

1247-
if (!sv)
1248+
if (!sv) {
1249+
watcher_attach(&ctx->srv_watch, px->srv);
12481250
sv = px->srv;
1251+
}
12491252

12501253
while (sv) {
12511254
labels[lb_idx].name = ist("server");
@@ -1397,10 +1400,11 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
13971400
&val, labels, &out, max))
13981401
goto full;
13991402
next_sv:
1400-
sv = sv->next;
1403+
sv = watcher_next(&ctx->srv_watch, sv->next);
14011404
}
14021405

14031406
next_px:
1407+
watcher_detach(&ctx->srv_watch);
14041408
px = px->next;
14051409
}
14061410
ctx->flags |= PROMEX_FL_METRIC_HDR;
@@ -1451,8 +1455,10 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
14511455
if ((px->flags & PR_FL_DISABLED) || px->uuid <= 0 || !(px->cap & PR_CAP_BE))
14521456
goto next_px2;
14531457

1454-
if (!sv)
1458+
if (!sv) {
1459+
watcher_attach(&ctx->srv_watch, px->srv);
14551460
sv = px->srv;
1461+
}
14561462

14571463
while (sv) {
14581464
labels[lb_idx].name = ist("server");
@@ -1477,10 +1483,11 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
14771483
goto full;
14781484

14791485
next_sv2:
1480-
sv = sv->next;
1486+
sv = watcher_next(&ctx->srv_watch, sv->next);
14811487
}
14821488

14831489
next_px2:
1490+
watcher_detach(&ctx->srv_watch);
14841491
px = px->next;
14851492
}
14861493
ctx->flags |= PROMEX_FL_METRIC_HDR;
@@ -1500,11 +1507,6 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
15001507
return -1; /* Unexpected and unrecoverable error */
15011508
}
15021509

1503-
/* Decrement server refcount if it was saved through ctx.p[1]. */
1504-
srv_drop(ctx->p[1]);
1505-
if (sv)
1506-
srv_take(sv);
1507-
15081510
/* Save pointers (0=current proxy, 1=current server, 2=current stats module) of the current context */
15091511
ctx->p[0] = px;
15101512
ctx->p[1] = sv;
@@ -2027,6 +2029,7 @@ static int promex_appctx_init(struct appctx *appctx)
20272029
LIST_INIT(&ctx->modules);
20282030
ctx->filters = EB_ROOT;
20292031
appctx->st0 = PROMEX_ST_INIT;
2032+
watcher_init(&ctx->srv_watch, &ctx->p[1], offsetof(struct server, watcher_list));
20302033
return 0;
20312034
}
20322035

@@ -2040,10 +2043,8 @@ static void promex_appctx_release(struct appctx *appctx)
20402043
struct promex_metric_filter *flt;
20412044
struct eb32_node *node, *next;
20422045

2043-
if (appctx->st1 == PROMEX_DUMPER_SRV) {
2044-
struct server *srv = objt_server(ctx->p[1]);
2045-
srv_drop(srv);
2046-
}
2046+
if (appctx->st1 == PROMEX_DUMPER_SRV)
2047+
watcher_detach(&ctx->srv_watch);
20472048

20482049
list_for_each_entry_safe(ref, back, &ctx->modules, list) {
20492050
LIST_DELETE(&ref->list);

0 commit comments

Comments
 (0)