Hi, Here are two small config parsing fixes against current master (3e758361c). Each one has a reg-test under reg-tests/startup/. The patches are attached (git format-patch).
1/2 BUG/MINOR: thread: don't turn reversed thread ranges into "even" parse_thread_set() does not check that a range's last thread is >= its first one, and max < min is how "all"/"odd"/"even" are stored. So "bind ... thread 4-2" (or "1/4-2", or "4-1") is accepted silently and gives the "even" mask. With nbthread 4, "show fd" reports tmask=0xa for "thread 4-2" and tmask=0xe for "thread 2-4". The patch swaps a reversed range, as cpu-map and "process" already do, so existing configs keep working with the threads they name. If you would prefer an error, it is a two-line change and I can send a v2. 2/2 BUG/MINOR: tools: make strl2irc() and strl2llrc() reject a lone minus sign A lone "-" parses as 0. "uid -" / "gid -" silently set root, with no "not a number" warning and no "running as root" warning. Also "set server b/s check-port -" sets port 0, req.hdr_ip() accepts "1.2.3.4:-", the ACL pattern ":-" means ":0", and "set table ... data.gpc0 -" stores 0. With the patch, all of these give the existing error or warning. This is a different case from the strl2ic() fix in GitHub #3357. The port_only v2 series currently on the list also uses strl2irc(), so it would inherit this case. Testing (Docker, Ubuntu 24.04, gcc 13, TARGET=linux-glibc USE_OPENSSL=1 USE_ZLIB=1 USE_PCRE2=1): the new reg-tests fail on master and pass with the patches. The full "make reg-tests" run gives 317 passed and 1 failed with the patches, against 315 passed and 3 failed on master. The 2 extra failures on master are the new tests, and the remaining failure (ssl/wrong_ctx_storage.vtc) is environmental and happens on both. Both fixes should be safe to backport (1/2 down to 2.8, where the thread_set parser appeared). This issue was found by an LLM-Enhanced Static Analysis Engine and verified manually. Thanks, Martin Mirchev
From 4a8770c77a45bedf14d83a05141c3c54aee50620 Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Tue, 27 Oct 2026 11:36:58 +0800 Subject: [PATCH 1/2] BUG/MINOR: thread: don't turn reversed thread ranges into "even" parse_thread_set() checks that the last thread number of a range is within the permitted limits, but not that it is greater than or equal to the first one. When storing the set, "max < min" is the internal encoding of "all" (max=0), "odd" (max=-1) and "even" (max=-2), so any range written in reverse order such as "thread 4-2" or "thread 1/4-2" on a "bind" line was silently accepted and stored as "even" (memset 0xaa). With "nbthread 4", "thread 4-2" binds the listener to threads 2 and 4 instead of threads 2 to 4, and "thread 4-1" also gives threads 2 and 4. There is no warning, so the only visible effect is that some threads never accept connections on that listener, which is easy to miss and hard to diagnose. The other range parsers ("cpu-map", "process" via parse_process_number(), and the cpuset parser) swap a reversed range, so let's do the same here. This keeps configurations that rely on such ranges working, with the threads the user wrote. A reg-test is added to check that "2-4", "4-2" and "1/4-2" give the same thread mask. This bug was introduced with the thread_set API in 2.8 (commit bef43dfa6 "MINOR: thread: add a simple thread_set API"), so this fix may be backported to all stable branches down to 2.8. --- .../startup/bind_thread_reverse_range.vtc | 38 +++++++++++++++++++ src/thread.c | 11 ++++++ 2 files changed, 49 insertions(+) create mode 100644 reg-tests/startup/bind_thread_reverse_range.vtc diff --git a/reg-tests/startup/bind_thread_reverse_range.vtc b/reg-tests/startup/bind_thread_reverse_range.vtc new file mode 100644 index 000000000..fdf60597e --- /dev/null +++ b/reg-tests/startup/bind_thread_reverse_range.vtc @@ -0,0 +1,38 @@ +varnishtest "bind 'thread' ranges given in reverse order" + +# A thread range whose last number is lower than the first one (e.g. "4-2") +# used to be stored as the "even" thread set. It must cover the same threads +# as the range written in ascending order, as done for "cpu-map". + +feature cmd "$HAPROXY_PROGRAM -cc 'feature(THREAD)'" +feature ignore_unknown_macro + +haproxy h1 -conf { + global + nbthread 4 + thread-groups 1 + + defaults + mode http + timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" + timeout client "${HAPROXY_TEST_TIMEOUT-5s}" + timeout server "${HAPROXY_TEST_TIMEOUT-5s}" + + frontend fwd + bind "fd@${fwd}" thread 2-4 + + frontend rev + bind "fd@${rev}" thread 4-2 + + frontend relrev + bind "fd@${relrev}" thread 1/4-2 +} -start + +haproxy h1 -cli { + send "show fd" + expect ~ "tmask=0xe [^\n]* fe=fwd " + send "show fd" + expect ~ "tmask=0xe [^\n]* fe=rev " + send "show fd" + expect ~ "tmask=0xe [^\n]* fe=relrev " +} diff --git a/src/thread.c b/src/thread.c index aacae3c26..56b1dc2cb 100644 --- a/src/thread.c +++ b/src/thread.c @@ -2057,6 +2057,17 @@ int parse_thread_set(const char *arg, struct thread_set *ts, char **err) set, is_rel ? MAX_THREADS_PER_GROUP : MAX_THREADS); return -1; } + + /* a range given in reverse order is accepted and + * swapped, as done for "cpu-map" and "process". + * Otherwise max < min would be stored as "all", + * "odd" or "even" below. + */ + if (max < min) { + v = min; + min = max; + max = v; + } } /* here sep points to the first non-digit after the thread spec, -- 2.50.1
From b5582beb3d8f33e3d01af13fe80f81cf894cc241 Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Tue, 27 Oct 2026 11:36:58 +0800 Subject: [PATCH 2/2] BUG/MINOR: tools: make strl2irc() and strl2llrc() reject a lone minus sign strl2irc() and strl2llrc() are documented to accurately detect any error (truncated string, invalid chars, overflows). But when the string is made of a single '-', the negative branch skips the sign, the digit loop does not run since there is nothing left, and the function returns success with 0. As a result, a lone "-" is silently taken as 0 wherever these functions are used to validate a number: - "uid -" and "gid -" in the global section set uid/gid 0 (root) instead of emitting the usual "is not a number" warning. When started as root, this also suppresses the warning about running as root without "user"/"uid", so the process silently keeps root privileges; - "set server <b>/<s> check-port -" (and "agent-port -") on the CLI sets port 0 instead of failing with "provided port is not an integer"; - req.hdr_ip() accepts "1.2.3.4:-" as a valid address and port; - an integer ACL pattern ":-" is taken as ":0", and a "-" value in the CLI "set table ... data.<type> -" command stores 0. Let's return an error when the sign is not followed by any digit. A reg-test checks that "uid -" and "gid -" now produce the warning (made fatal with -dW). This has been there since the functions were introduced, so the fix may be backported to all stable branches. --- reg-tests/startup/uid_gid_sign_only.vtc | 22 ++++++++++++++++++++++ src/tools.c | 4 ++++ 2 files changed, 26 insertions(+) create mode 100644 reg-tests/startup/uid_gid_sign_only.vtc diff --git a/reg-tests/startup/uid_gid_sign_only.vtc b/reg-tests/startup/uid_gid_sign_only.vtc new file mode 100644 index 000000000..5e55f76ce --- /dev/null +++ b/reg-tests/startup/uid_gid_sign_only.vtc @@ -0,0 +1,22 @@ +varnishtest "a lone '-' is not a valid number for uid/gid" + +# strl2irc() used to return 0 for a lone '-', so "uid -" or "gid -" silently +# meant 0 (root) instead of emitting the "is not a number" warning. The +# warning is made fatal with -dW so that the config check fails. + +feature ignore_unknown_macro + +haproxy h1 -arg -dW -conf-BAD {} { + global + uid - +} + +haproxy h2 -arg -dW -conf-BAD {} { + global + gid - +} + +haproxy h3 -arg -dW -conf-BAD {} { + global + uid -x +} diff --git a/src/tools.c b/src/tools.c index 16936985b..11ad0f03a 100644 --- a/src/tools.c +++ b/src/tools.c @@ -2821,6 +2821,8 @@ int strl2irc(const char *s, int len, int *ret) } } else { /* negative number */ + if (len == 1) + return 1; /* sign alone, no digit */ s++; while (--len > 0) { j = (*s++) - '0'; @@ -2865,6 +2867,8 @@ int strl2llrc(const char *s, int len, long long *ret) } } else { /* negative number */ + if (len == 1) + return 1; /* sign alone, no digit */ s++; while (--len > 0) { j = (*s++) - '0'; -- 2.50.1
