From e0e2b6613214212332de4cbad2fc06cf4774c1b0 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 16 Jul 2024 11:14:49 +0200 Subject: [PATCH] BUG/MEDIUM: debug/cli: fix "show threads" crashing with low thread counts The "show threads" command introduced early in the 2.0 dev cycle uses appctx->st1 to store its context (the number of the next thread to dump). It goes back to an era where contexts were shared between the various applets and the CLI's command handlers. In fact it was already not good by then because st1 could possibly have APPCTX_CLI_ST1_PAYLOAD (2) in it, that would make the dmup start at thread 2, though it was extremely unlikely. When contexts were finally cleaned up and moved to their own storage, this one was overlooked, maybe due to using st1 instead of st2 like most others. So it continues to rely on st1, and more recently some new flags were appended, one of which is APPCTX_CLI_ST1_LASTCMD (16) and is always there. This results in "show threads" to believe it must start do dump from thread 16, and if this thread is not present, it can simply crash the process. A tiny reproducer is: global nbthread 1 stats socket /tmp/sock1 level admin mode 666 $ socat /tmp/sock1 - <<< "show threads" The fix for modern versions simply consists in assigning a context to this command from the applet storage. We're using a single int, no need for a struct, an int* will do it. That's valid till 2.6. Prior to 2.6, better switch to appctx->ctx.cli.i0 or i1 which are all properly initialized before the command is executed. This must be backported to all stable versions. Thanks to Andjelko Horvat for the report and the reproducer. --- src/debug.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/debug.c b/src/debug.c index dfd44f5a2..679a4e127 100644 --- a/src/debug.c +++ b/src/debug.c @@ -470,24 +470,21 @@ void ha_task_dump(struct buffer *buf, const struct task *task, const char *pfx) */ static int cli_io_handler_show_threads(struct appctx *appctx) { - int thr; + int *thr = appctx->svcctx; - if (appctx->st0) - thr = appctx->st1; - else - thr = 0; + if (!thr) + thr = applet_reserve_svcctx(appctx, sizeof(*thr)); do { chunk_reset(&trash); - ha_thread_dump(&trash, thr); + ha_thread_dump(&trash, *thr); if (applet_putchk(appctx, &trash) == -1) { /* failed, try again */ - appctx->st1 = thr; return 0; } - thr++; - } while (thr < global.nbthread); + (*thr)++; + } while (*thr < global.nbthread); return 1; }