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 071ae8ce3 (" BUG/MEDIUM: stats/server: use
watcher to track server during stats dump") was (up to 2.8 it seems)

diff --git a/addons/promex/service-prometheus.c b/addons/promex/service-prometheus.c
index 60f927553f0bc97e6cb15d03d3464aa53127eb48..25611c400653eb209f414206ad54be438d3268c4 100644 (file)
--- a/addons/promex/service-prometheus.c
+++ b/addons/promex/service-prometheus.c
@@ -82,6 +82,7 @@ struct promex_ctx {
        unsigned field_num;        /* current field number (ST_I_PX_* etc) */
        unsigned mod_field_num;    /* first field number of the current module (ST_I_PX_* etc) */
        int obj_state;             /* current state among PROMEX_{FRONT|BACK|SRV|LI}_STATE_* */
+       struct watcher srv_watch;  /* watcher to automatically update next pointer */
        struct list modules;       /* list of promex modules to export */
        struct eb_root filters;    /* list of filters to apply on metrics name */
 };
@@ -1244,8 +1245,10 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
                        if ((px->flags & PR_FL_DISABLED) || px->uuid <= 0 || !(px->cap & PR_CAP_BE))
                                goto next_px;
 
-                       if (!sv)
+                       if (!sv) {
+                               watcher_attach(&ctx->srv_watch, px->srv);
                                sv = px->srv;
+                       }
 
                        while (sv) {
                                labels[lb_idx].name  = ist("server");
@@ -1397,10 +1400,11 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
                                                    &val, labels, &out, max))
                                        goto full;
                          next_sv:
-                               sv = sv->next;
+                               sv = watcher_next(&ctx->srv_watch, sv->next);
                        }
 
                  next_px:
+                       watcher_detach(&ctx->srv_watch);
                        px = px->next;
                }
                ctx->flags |= PROMEX_FL_METRIC_HDR;
@@ -1451,8 +1455,10 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
                                if ((px->flags & PR_FL_DISABLED) || px->uuid <= 0 || !(px->cap & PR_CAP_BE))
                                        goto next_px2;
 
-                               if (!sv)
+                               if (!sv) {
+                                       watcher_attach(&ctx->srv_watch, px->srv);
                                        sv = px->srv;
+                               }
 
                                while (sv) {
                                        labels[lb_idx].name  = ist("server");
@@ -1477,10 +1483,11 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
                                                goto full;
 
                                  next_sv2:
-                                       sv = sv->next;
+                                       sv = watcher_next(&ctx->srv_watch, sv->next);
                                }
 
                          next_px2:
+                               watcher_detach(&ctx->srv_watch);
                                px = px->next;
                        }
                        ctx->flags |= PROMEX_FL_METRIC_HDR;
@@ -1500,11 +1507,6 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
                        return -1; /* Unexpected and unrecoverable error */
        }
 
-       /* Decrement server refcount if it was saved through ctx.p[1]. */
-       srv_drop(ctx->p[1]);
-       if (sv)
-               srv_take(sv);
-
        /* Save pointers (0=current proxy, 1=current server, 2=current stats module) of the current context */
        ctx->p[0] = px;
        ctx->p[1] = sv;
@@ -2027,6 +2029,7 @@ static int promex_appctx_init(struct appctx *appctx)
        LIST_INIT(&ctx->modules);
        ctx->filters = EB_ROOT;
        appctx->st0 = PROMEX_ST_INIT;
+       watcher_init(&ctx->srv_watch, &ctx->p[1], offsetof(struct server, watcher_list));
        return 0;
 }
 
@@ -2040,10 +2043,8 @@ static void promex_appctx_release(struct appctx *appctx)
        struct promex_metric_filter *flt;
         struct eb32_node *node, *next;
 
-       if (appctx->st1 == PROMEX_DUMPER_SRV) {
-               struct server *srv = objt_server(ctx->p[1]);
-               srv_drop(srv);
-       }
+       if (appctx->st1 == PROMEX_DUMPER_SRV)
+               watcher_detach(&ctx->srv_watch);
 
        list_for_each_entry_safe(ref, back, &ctx->modules, list) {
                LIST_DELETE(&ref->list);