From: Yeonggi Kim <[email protected]>
Since 20ec1de214 ("MAJOR: cli: Refacor parsing and execution of
pipelined commands"), a whole command line is copied into a chunk before
its commands are executed, and the last command is detected by comparing
its end with the tail of this chunk:
end_of_cmdline = (... : b_tail(appctx->cli_ctx.cmdline)-1);
...
if (end == end_of_cmdline) {
appctx->st1 |= APPCTX_CLI_ST1_LASTCMD;
The chunk is a trash chunk of tune.bufsize bytes, and b_getdelim() fills
it up to its last byte when the line, including its final LF, is exactly
tune.bufsize bytes long. b_tail() then wraps to the beginning of the
area, and the last command is never detected. Once it is executed, the
words of the line, separated by NUL bytes by the first parsing, are
parsed again one by one as if more commands were pending, and b_del()
removes more bytes than the chunk holds, which makes b_data() underflow.
This never ends: the CLI sends "Unknown command" errors, keeping a
thread busy for as long as the client reads them. A build with
DEBUG_STRICT=2 dies instead on the BUG_ON_HOT() in b_del(). With one
byte less, the line is processed normally, and with one byte more, it is
rejected as too big. The same happens when several commands separated
by semicolons fill the chunk, or with a line sent through the master CLI
which becomes exactly tune.bufsize bytes long once the master added its
prefixes (e.g. "expert-mode on -;").
Reproducer, with "tune.bufsize 1024" and a "stats socket":
printf 'show version%1011s\n' '' | socat - /tmp/haproxy.sock
The command line chunk is allocated empty by alloc_trash_chunk(), and is
only filled by cli_parse_cmdline(), from its beginning, before any of
its commands is processed. cli_process_cmdline() then only removes data
from its head. So the data never wrap, and the end of the command line
is now computed from its head and the amount of data, which also works
when the chunk is full.
This must be backported as far as 3.2. On 3.3 and 3.2, the end of the
command line is tested with "end+1 == b_tail(appctx->cli_ctx.cmdline)",
which must become "end+1 == orig + b_data(appctx->cli_ctx.cmdline)".
---
src/cli.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/cli.c b/src/cli.c
index 5bdc5a260..21a23a1fb 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -817,7 +817,13 @@ int cli_process_cmdline(struct appctx *appctx)
orig = p = b_head(appctx->cli_ctx.cmdline);
end = p + strlen(p);
- end_of_cmdline = (appctx->cli_ctx.payload_pat ?
appctx->cli_ctx.payload_pat - strlen(PAYLOAD_PATTERN) :
b_tail(appctx->cli_ctx.cmdline)-1);
+ /* The command line chunk is filled from its beginning and never wraps.
+ * Don't use b_tail() to find its end, it wraps to the beginning of the
+ * area when the command line fills the whole chunk.
+ */
+ end_of_cmdline = (appctx->cli_ctx.payload_pat
+ ? appctx->cli_ctx.payload_pat -
strlen(PAYLOAD_PATTERN)
+ : orig + b_data(appctx->cli_ctx.cmdline) - 1);
/*
* Get pointers on words.
* One extra slot is reserved to store a pointer on a null byte.
--
2.52.0
