BUG/MINOR: http_ana: Report -1 for %Tr for invalid response only

The server response time is erroneously reported as -1 when it is
intercepted by HAProxy.

As stated in the documentation, the server response time is reported as -1
when the last response header was never seen. It happens when a server
timeout is triggered before the server managed to process the request. It
also happens if the response is invalid. This may be reported by the mux
during the response parsing, but also by the HTTP analyzers. However, in
this last case, the response time must only be reported as -1 on 502.

This patch must be backported to all stable versions. It should fix the
issue #2384.

(cherry picked from commit 5863d33fce702c46b77c07d4ea82e036b11417a6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: Fix a typo in "1.3.1. The Request line"

At the beginning of the last paragraph of this section, HTTP/3 was used
instead of HTTP/2. It is not fixed.

(cherry picked from commit 18de419f9647ad5fe0006900e2c1587bffd49c24)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: A a space before ':' for {bs,fs}.aborted and {bs,fs}.rst_code

A space was missing before the ':' for the sample fetch functions above. It
was an issue for the text to HTML conversion script. So, let's fix it.

(cherry picked from commit 3af2d91b3b6ebe1587bcb17f5fb223436df67253)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: peers: make sure to always apply offsets to now_ms in expiration

Now_ms can be zero nowadays, so it's not suitable for direct assignment to
t->expire, as there's a risk that the timer never wakes up once assigned
(TICK_ETERNITY). Let's use tick_add(now_ms, 0) for an immediate wakeup
instead. The impact here might be a reconnect programmed upon signal
receipt at the wrapping date not having a working timeout.

This should be backported where it applies.

(cherry picked from commit ed55ff878d5af35dae70f78023ab2141d36e5866)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mux_quic: make sure to always apply offsets to now_ms in expiration

Now_ms can be zero nowadays, so it's not suitable for direct assignment to
t->expire, as there's a risk that the timer never wakes up once assigned
(TICK_ETERNITY). Let's use tick_add(now_ms, 0) for an immediate wakeup
instead. The impact looks nul since the task is also woken up, but better
not leave such tasks in the timer tree anyway.

This should be backported where it applies.

(cherry picked from commit f66bfcff96082ce5c98c635c5da7a9ba157a20af)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mailers: make sure to always apply offsets to now_ms in expiration

Now_ms can be zero nowadays, so it's not suitable for direct assignment to
t->expire, as there's a risk that the timer never wakes up once assigned
(TICK_ETERNITY). Let's use tick_add(now_ms, 0) for an immediate wakeup
instead. The impact here might be mailers suddenly stopping.

This should be backported where it applies.

(cherry picked from commit 841be4cdd15b3d0834a478cc95ebda0f47171b4d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: checks: make sure to always apply offsets to now_ms in expiration

Now_ms can be zero nowadays, so it's not suitable for direct assignment to
t->expire, as there's a risk that the timer never wakes up once assigned
(TICK_ETERNITY). Let's use tick_add(now_ms, 0) for an immediate wakeup
instead. The impact here might be health checks suddenly stopping.

This should be backported where it applies.

(cherry picked from commit 2f287f14f355e734e512732e35aebf993d000792)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: Don't report early srv aborts on request forwarding in DONE state

L7-retries may be ignored if server aborts are detected during the request
forwarding, when the request is already in DONE state.

When a request was fully processed (so in HTTP_MSG_DONE state) and is
waiting for be forwarded to the server, there is a test to detect server
aborts, to be able to report the error. However, this test must be skipped
if the response was not received yet, to let the reponse analyszers handle
the abort. It is important to properly handle the retries. This test must
only be performed if the response analysis was finished. It means the
response must be at least in HTTP_MSG_BODY state.

This patch should be backported as far as 2.8.

(cherry picked from commit a930e99f4699676ea72f72ba1fb99c953da0d74e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-h2: Don't send RST_STREAM frame for streams with no ID

On server side, the H2 stream is first created with an unassigned ID (ID ==
0). Its ID is assigned when the request is emitted, before formatting the
HEADERS frame. However, the session may be aborted during that stage. We
must take care to not emit RST_STREAM frame for this stream, because it does
not exist yet for the server.

It is especially important to do so because, depending on the timing, it may
also happens before the H2 PREFACE was sent.

This patch must be backported to all stable versions. It is related to issue

(cherry picked from commit f065d0009888c394e5f93dfdaa2ae79958b2c2e2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: Insert a non-executed resulution in front of the wait list

When a resolver is woken up to process DNS resolutions, it is possible to
trigger an infinite loop on the resolver's wait list because delayed
resolutions are always reinserted at the end of this list. This leads the
watchdog to kill the process. By re-inserting them in front of the list,
that fixes the bug.

When a resolver tries to send the queries for the resolutions in its wait
list, it may be unable to proceed for a resolution. This may happen because
the resolution must be skipped (no hostname to resolv, a resolution already
in-progress) or when an error occurred. In that case, the resolution is
re-inserted in the resolver's wait list to be retry later, on a next wakeup.

However, the resolution is inserted at the end of the wait list. So it is
immediately reevaluated, in the same execution loop, instead of to be
delayed. Most of time, it is not an issue because the resolution is
considered as not expired on the second run. But it is an problem when the
internal time wraps and is equal to 0. In that case, the resolution
expiration date is badly computed and it is always considered as expired. If
two or more resolutions are in that state, the resolver loops for ever on
its wait list, until the process is killed by the watchdog.

So we can argue that the way the resolution expiration date is computed must
be fixed. And it would be true in a perfect world. However, the resolvers
code is so crapy that it is hard to be sure to not introduce regressions. It
is farly easier to re-insert delayed resolutions in front of the wait
list. This fixes the issue and at worst, these resolutions will be evaluated
one time too many on the next wakeup and only if now_ms was equal to 0 on
the prior wakeup.

This patch should be backported to all stable versions. On 2.2, LIST_ADD()
must be used instead of LIST_INSERT()

(cherry picked from commit 8f28dbeea94e11e2327362755f16d18b301fd153)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: cli: don't show sockpairs in HAPROXY_CLI and HAPROXY_MASTER_CLI

Before this fix, HAPROXY_CLI and HAPROXY_MASTER_CLI have contained along with
CLI sockets addresses internal sockpairs, which are used only for master CLI
(reload sockpair and sockpair shared with a worker process). These internal
sockpairs are always need to be hidden.

At the moment there is no any client, who uses sockpair addresses for the
stats listener or in order to connect to master CLI. So, let's simply not copy
these internal sockpair addresses of MASTER and GLOBAL proxy listeners.

As listeners with sockpairs are skipped and they can be presented in the
listeners list in any order, let's add semicolon separator between addresses
only in the case, when there are already some string saved in the trash and we
are sure, that we are adding a new address to it. Otherwise, we could have such
weird output:

HAPROXY_MASTER_CLI=unix@/tmp/mcli.sock;;

This fix is need to be backported in all stable versions.

(cherry picked from commit 113745e6f0c0ef8fe89e89fdfdcc6ed994889d4a)
[cf: ctx adjt]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: quic: prevent crash due to CRYPTO parsing error

A packet which contains several splitted and out of order CRYPTO frames
may be parsed multiple times to ensure it can be handled via ncbuf. Only
3 iterations can be performed to prevent excessive CPU usage.

There is a risk of crash if packet parsing is interrupted after maximum
iterations is reached, or no progress can be made on the ncbuf. This is
because <frm> may be dangling after list_for_each_entry_safe()

The crash occurs on qc_frm_free() invokation, on error path of
qc_parse_pkt_frms(). To fix it, always reset frm to NULL after
list_for_each_entry_safe() to ensure it is not dangling.

This should fix new report on github isue #2776. This regression has
been triggered by the following patch :
  1767196d5b2d8d1e557f7b3911a940000166ecda
  BUG/MINOR: quic: repeat packet parsing to deal with fragmented CRYPTO

As such, it must be backported up to 2.6, after the above patch.

(cherry picked from commit 2975e8805d9e84010bf5199a2365d650923dbb2c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: guid/server: ensure thread-safety on GUID insert/delete

Since 3.0, it is possible to assign a GUID to proxies, listeners and
servers. These objects are stored in a global tree guid_tree.

Proxies and listeners are static. However, servers may be added or
deleted at runtime, which imply that guid_tree must be protected. Fix
this by declaring a read-write lock to protect tree access.

For now, only guid_insert() and guid_remove() are protected using a
write lock. Outside of these, GUID tree is not accessed at runtime. If
server CLI commands are extended to support GUID as server identifier,
lookup operation should be extended with a read lock protection.

Note that during stat-file preloading, GUID tree is accessed for lookup.
However, as it is performed on startup which is single threaded, there
is no need for lock here. A BUG_ON() has been added to ensure this
precondition remains true.

This bug could caused a segfault when using dynamic servers with GUID.
However, it was never reproduced for now.

This must be backported up to 3.0. To avoid a conflict issue, the
previous cleanup patch can be merged before it.

(cherry picked from commit 8e0e7d9d1af5b2dfec2e625d2c19dd034c36eb04)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: guid: remove global tree export

guid_tree is not directly used outside of functions provided by the guid
module. Remove its export from the include file.

(cherry picked from commit b70880cdc9c01602197fd124c84ab264f6b4ddfb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: quic: repeat packet parsing to deal with fragmented CRYPTO

A ClientHello may be splitted accross several different CRYPTO frames,
then mixed in a single QUIC packet. This is used notably by clients such
as chrome to render the first Initial packet opaque to middleboxes.

Each packet frame is handled sequentially. Out-of-order CRYPTO frames
are buffered in a ncbuf, until gaps are filled and data is transferred
to the SSL stack. If CRYPTO frames are heavily splitted with small
fragments, buffering may fail as ncbuf does not support small gaps. This
causes the whole packet to be rejected and unacknowledged. It could be
solved if the client reemits its ClientHello after remixing its CRYPTO
frames.

This patch is written to improve CRYPTO frame parsing. Each CRYPTO
frames which cannot be buffered due to ncbuf limitation are now stored
in a temporary list. Packet parsing is completed until all frames have
been handled. If temporary list is not empty, reparsing is done on the
stored frames. With the newly buffered CRYPTO frames, ncbuf insert
operation may this time succeeds if the frame now covers a whole gap.
Reparsing will loop until either no progress can be made or it has been
done at least 3 times, to prevent CPU utilization.

This patch should fix github issue #2776.

This should be backported up to 2.6, after a period of observation. Note
that it relies on the following refactor patches :
  MINOR: quic: extend return value of CRYPTO parsing
  MINOR: quic: use dynamically allocated frame on parsing
  MINOR: quic: simplify qc_parse_pkt_frms() return path

(cherry picked from commit 1767196d5b2d8d1e557f7b3911a940000166ecda)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: quic: extend return value of CRYPTO parsing

qc_handle_crypto_frm() is the function used to handled a newly received
CRYPTO frame. Change its API to use a newly dedicated return type. This
allows to report if the frame was properly handled, ignored if already
parsed previously or rejected after a fatal error.

This commit does not have any functional changes. However, it allows to
simplify qc_handle_crypto_frm() API by removing <fast_retrans> as output
parameter. Also, this patch will be necessary to support multiple
iteration of packet parsing for CRYPTO frames.

(cherry picked from commit d65e782c8cd2f8554404dd1424e2d64f3786edb1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: quic: use dynamically allocated frame on parsing

qc_parse_pkt_frms() is the function responsible to parse a received QUIC
packet. Payload is decoded and splitted into individual frames which are
then handled individually. Previously, frame was used as locally stack
allocated. Change this to work on a dynamically allocated frame.

This commit does bring any functional changes. However, it will be
useful to extend packet parsing. In particular, it will be necessary to
save some frames during parsing to reparse them after the others.

(cherry picked from commit 190fc97606560568bf4a611d92c1e70aed057843)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: quic: simplify qc_parse_pkt_frms() return path

Change qc_parse_pkt_frms() return path for normal and error cases. Most
notably, it allows to remove local variable ret as now return value is
hardcoded on normal and err label. This also allows to define a
different trace for error leaving code.

(cherry picked from commit 498a99a84956535a9ce2a61cb908d0fc81165606)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: quic: support wait-for-handshake

wait-for-handshake http-request action was completely ineffective with
QUIC protocol. This commit implements its support for QUIC.

QUIC MUX layer is extended to support wait-for-handshake. A new function
qcc_handle_wait_for_hs() is executed during qcc_io_process(). It detects
if MUX processing occurs after underlying QUIC handshake completion. If
this is the case, it indicates that early data may be received. As such,
connection is flagged with CO_FL_EARLY_SSL_HS, which is necessary to
block stream processing on wait-for-handshake action.

After this, qcc subscribs on quic_conn layer for RECV notification. This
is used to detect QUIC handshake completion. Thus,
qcc_handle_wait_for_hs() can be reexecuted one last time, to remove
CO_FL_EARLY_SSL_HS and notify every streams flagged as
SE_FL_WAIT_FOR_HS.

This patch must be backported up to 2.6, after a mandatory period of
observation. Note that it relies on the backport of the two previous
patches :
- MINOR: quic: notify connection layer on handshake completion
- BUG/MINOR: stream: unblock stream on wait-for-handshake completion

(cherry picked from commit 0918c41ef63964a986c627d20b8a1324de639cc2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: stream: unblock stream on wait-for-handshake completion

wait-for-handshake is an http-request action which permits to delay the
processing of content received as TLS early data. The action yields
as long as connection handshake is in progress. In the meantime, stconn
is flagged with SE_FL_WAIT_FOR_HS.

When the handshake is finished, MUX layer is responsible to woken up
SE_FL_WAIT_FOR_HS flagged stconn instances to restart the stream
processing. On sc_conn_process(), SE_FL_WAIT_FOR_HS flag is removed and
stream layer is woken up.

However, there may be a blocking after MUX notification. sc_conn_recv()
may return 0 due to no new data reception, which prevents
sc_conn_process() execution. The stream is thus blocked until its
timeout.

To fix this, checks in sc_conn_recv() about the handshake termination
condition. If true, explicitely returns 1 to ensure sc_conn_process()
will be executed.

Note that this bug is not reproducible due to various conditions related
to early data implementation in haproxy. Indeed, connection layer
instantiation is always delayed until SSL handshake completion, which
prevents the handling of early data as expected.

This fix will be necessary to implement wait-for-handshake support for
QUIC. As such, it must be backported with the next commit up to 2.6,
after a mandatory period of observation.

(cherry picked from commit 73031e81cdd5cf5ba889ed4c676a4ae6284f5cf6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: quic: notify connection layer on handshake completion

Wake up connection layer on QUIC handshake completion via
quic_conn_io_cb. Select SUB_RETRY_RECV as this was previously unused by
QUIC MUX layer.

For the moment, QUIC MUX never subscribes for handshake completion.
However, this will be necessary for features such as the delaying of
early data forwarding via wait-for-handshake.

This patch will be necessary to implement wait-for-handshake support for
QUIC. As such, it must be backported with next commits up to 2.6,
after a mandatory period of observation.

(cherry picked from commit 5a5950e42d7060ee311e51438f4f16ad0effefd9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: pattern: prevent uninitialized reads in pat_match_{str,beg}

Using valgrind when running map_beg or map_str, the following error is
reported:

==242644== Conditional jump or move depends on uninitialised value(s)
==242644==    at 0x2E4AB1: pat_match_str (pattern.c:457)
==242644==    by 0x2E81ED: pattern_exec_match (pattern.c:2560)
==242644==    by 0x343176: sample_conv_map (map.c:211)
==242644==    by 0x27522F: sample_process_cnv (sample.c:1330)
==242644==    by 0x2752DB: sample_process (sample.c:1373)
==242644==    by 0x319917: action_store (vars.c:814)
==242644==    by 0x24D451: http_req_get_intercept_rule (http_ana.c:2697)

In fact, the error is legit, because in pat_match_{beg,str}, we
dereference the buffer on len+1 to check if a value was previously set,
and then decide to force NULL-byte if it wasn't set.

But the approach is no longer compatible with current architecture:
data past str.data is not guaranteed to be initialized in the buffer.
Thus we cannot dereference the value, else we expose us to uninitialized
read errors. Moreover, the check is useless, because we systematically
set the ending byte to 0 when the conditions are met.

Finally, restoring the older value after the lookup is not relevant:
indeed, either the sample is marked as const and in such case it
is already duplicated, or the sample is not const and we forcefully add
a terminating NULL byte outside from the actual string bytes (since we're
past str.data), so as we didn't alter effective string data and that data
past str.data cannot be dereferenced anyway as it isn't guaranteed to be
initialized, there's no point in restoring previous uninitialized data.

It could be backported in all stable versions. But since this was only
detected by valgrind and isn't known to cause issues in existing
deployments, it's probably better to wait a bit before backporting it
to avoid any breakage.. although the fix should be theoretically harmless.

(cherry picked from commit 8157c1caf26618d77b32be7906e4b608a8c0729b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

[RELEASE] Released version 3.0.6

Released version 3.0.6 with the following main changes :
    - MINOR: connection: No longer include stconn type header in connection-t.h
    - BUG/MINOR: h1: do not forward h2c upgrade header token
    - BUG/MINOR: h2: reject extended connect for h2c protocol
    - MINOR: mux-h1: Set EOI on SE during demux when both side are in DONE state
    - BUG/MEDIUM: mux-h1/mux-h2: Reject upgrades with payload on H2 side only
    - REGTESTS: h1/h2: Update script testing H1/H2 protocol upgrades
    - REGTESTS: shorten a bit the delay for the h1/h2 upgrade test
    - BUG/MINOR: mux-quic: report glitches to session
    - BUG/MEDIUM: cli: Be sure to catch immediate client abort
    - BUG/MEDIUM: cli: Deadlock when setting frontend maxconn
    - BUG/MINOR: server: make sure the HMAINT state is part of MAINT
    - BUG/MINOR: cfgparse-global: fix allowed args number for setenv
    - BUILD: tools: only include execinfo.h for the real backtrace() function
    - MINOR: tools: do not attempt to use backtrace() on linux without glibc
    - MINOR: task: define two new one-shot events for use with WOKEN_OTHER or MSG
    - BUG/MEDIUM: stream: make stream_shutdown() async-safe
    - BUG/MINOR: queue: make sure that maintenance redispatches server queue
    - MINOR: server: make srv_shutdown_sessions() call pendconn_redistribute()
    - BUG/MEDIUM: queue: always dequeue the backend when redistributing the last server
    - BUG/MINOR: mux-h1: Fix condition to set EOI on SE during zero-copy forwarding
    - BUG/MINOR: http-ana: Disable fast-fwd for unfinished req waiting for upgrade
    - MINOR: debug: make mark_tainted() return the previous value
    - MINOR: chunk: drop the global thread_dump_buffer
    - MINOR: debug: split ha_thread_dump() in two parts
    - MINOR: debug: slightly change the thread_dump_pointer signification
    - MINOR: debug: make ha_thread_dump_done() take the pointer to be used
    - MINOR: debug: replace ha_thread_dump() with its two components
    - MEDIUM: debug: on panic, make the target thread automatically allocate its buf
    - BUG/MEDIUM: server: server stuck in maintenance after FQDN change
    - BUG/MEDIUM: hlua: make hlua_ctx_renew() safe
    - BUG/MEDIUM: hlua: properly handle sample func errors in hlua_run_sample_{fetch,conv}()
    - BUG/MEDIUM: mux-quic: ensure timeout server is active for short requests
    - BUG/MEDIUM: queue: make sure never to queue when there's no more served conns
    - BUG/MINOR: httpclient: return NULL when no proxy available during httpclient_new()
    - BUG/MEDIUM: stconn: Wait iobuf is empty to shut SE down during a check send
    - BUG/MINOR: http-ana: Don't report a server abort if response payload is invalid
    - BUG/MEDIUM: stconn: Check FF data of SC to perform a shutdown in sc_notify()
    - BUG/MAJOR: filters/htx: Add a flag to state the payload is altered by a filter
    - REGTESTS: Never reuse server connection in http-messaging/truncated.vtc
    - BUG/MINOR: quic: avoid leaking post handshake frames
    - BUG/MEDIUM: quic: avoid freezing 0RTT connections
    - DOC: config: fix rfc7239 forwarded typo in desc
    - BUG/MINOR: mworker: fix mworker-max-reloads parser
    - BUG/MINOR: mux-quic: do not close STREAM with empty FIN if no data sent
    - BUG/MEDIUM: stats-html: Never dump more data than expected during 0-copy FF
    - BUG/MEDIUM: mux-h2: Remove H2S from send list if data are sent via 0-copy FF
    - BUG/MEDIUM: connection/http-reuse: fix address collision on unhandled address families
    - MINOR: activity/memprofile: always return "other" bin on NULL return address
    - MINOR: activity/memprofile: show per-DSO stats
    - BUG/MINOR: server: fix dynamic server leak with check on failed init
    - BUG/MEDIUM: stconn: Report blocked send if sends are blocked by an error
    - BUG/MINOR: http-ana: Fix wrong client abort reports during responses forwarding
    - BUG/MINOR: stconn: Don't disable 0-copy FF if EOS was reported on consumer side
    - BUG/MEDIUM: server: fix race on servers_list during server deletion
    - BUILD: debug: silence a build warning with threads disabled
    - MINOR: pools: export the pools variable
    - MINOR: debug: place a magic pattern at the beginning of post_mortem
    - MINOR: debug: place the post_mortem struct in its own section.
    - MINOR: debug: store important pointers in post_mortem
    - MINOR: cli: remove non-printable characters from 'debug dev fd'
    - BUG/MINOR: trace: stop rewriting argv with -dt
    - BUG/MINOR: ssl/cli: 'set ssl cert' does not check the transaction name correctly
    - DOC: config: add missing glitch_{cnt,rate} data types
    - DOC: config: add missing glitch_{cnt,rate} sample definitions
    - BUG/MEDIUM: mux-h1: Fix how timeouts are applied on H1 connections
    - BUG/MINOR: http-ana: Report internal error if an action yields on a final eval
    - MINOR: stream: Save last evaluated rule on invalid yield
    - BUG/MEDIUM: promex: Fix dump of extra counters
    - DOC: config: document connection error 44 (reverse connect failure)
    - CLEANUP: connection: properly name the CO_ER_SSL_FATAL enum entry
    - BUG/MINOR: quic: fix malformed probing packet building
    - MINOR: cli/debug: show dev: add cmdline and version
    - MINOR: stream/stats: Expose the current number of streams in stats
    - MINOR: stream/stats: Expose the total number of streams ever created in stats
    - BUG/MINOR: stats: Fix the name for the total number of streams created
    - MINOR: connection: add more connection error codes to cover common errno
    - MINOR: rawsock: set connection error codes when returning from recv/send/splice
    - MINOR: connection: add new sample fetch functions fc_err_name and bc_err_name
    - MINOR: debug: print gdb hints when crashing
    - MINOR: debug: do not limit backtraces to stuck threads
    - MINOR: debug: also add a pointer to struct global to post_mortem
    - MINOR: debug: also add fdtab and acitvity to struct post_mortem
    - MINOR: debug: remove the redundant process.thread_info array from post_mortem
    - MINOR: wdt: move the local timers to a struct
    - MINOR: debug: add a function to dump a stuck thread
    - DEBUG: wdt: better detect apparently locked up threads and warn about them
    - DEBUG: cli: make it possible for "debug dev loop" to trigger warnings
    - DEBUG: wdt: make the blocked traffic warning delay configurable
    - DEBUG: wdt: add a stats counter "BlockedTrafficWarnings" in show info
    - BUILD: debug: also declare strlen() in __ABORT_NOW()
    - BUILD: Missing inclusion header for ssize_t type
    - MINOR: debug: move the "recover now" warn message after the optional notes

MINOR: debug: move the "recover now" warn message after the optional notes

At the end of the too long processing warning added by commit 0950778b3a
("MINOR: debug: add a function to dump a stuck thread"), there can be some
optional notes about lua and memory trimming. However it's a bit awkward
that they appear after the "trying to recover now" message. Let's just move
that message after the notes.

(cherry picked from commit 5dcf2012fc035e790c118590a12240e0769fbcaa)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUILD: Missing inclusion header for ssize_t type

Compilation issue detected as follows by gcc:

In file included from src/ncbuf.c:19:
src/ncbuf.c: In function 'ncb_write_off':
include/haproxy/bug.h:144:10: error: unknown type name 'ssize_t'
  144 |   extern ssize_t write(int, const void *, size_t); \

(cherry picked from commit bc9821fd26b3a118415f579cdfa6e430b03f96da)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUILD: debug: also declare strlen() in __ABORT_NOW()

Previous commit 8f204fa8ae ("MINOR: debug: print gdb hints when crashing")
broken on the CI where strlen() isn't known. Let's forward-declare it in
the __ABORT_NOW() functions, just like write(). No backport is needed.

(cherry picked from commit 2d27c80288c0acee85326c0574ed70d0b2e486ef)
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: wdt: add a stats counter "BlockedTrafficWarnings" in show info

Every time a warning is issued about traffic being blocked, let's
increment a global counter so that we can check for this situation
in "show info".

(cherry picked from commit 84dd05e7d83eeee4e7b8c64dc656cdd608c78806)
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: wdt: make the blocked traffic warning delay configurable

The new global "warn-blocked-traffic-after" allows one to configure
after how much time a warning should be emitted when traffic is blocked.

(cherry picked from commit 6127e5a4e9722c1b47f5a9810fd41892b675557b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: cli: make it possible for "debug dev loop" to trigger warnings

A new argument "warn" allows to force the emission of a warning while
stuck in the loop by making the internal state inconsistent.

(cherry picked from commit 7337c422247b7af342048cfd48ac0aa2a4b7335e)
[wt: backported only to help testing the watchdog backports]
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: wdt: better detect apparently locked up threads and warn about them

In order to help users detect when threads are behaving abnormally, let's
try to emit a warning when one is no longer making any progress. This will
allow to catch faulty situations more accurately, instead of occasionally
triggering just after the long task. It will also let users know that there
is something wrong with their configuration, and inspect the call trace to
figure whether they're using excessively long rules or Lua for example (the
usual warnings about lua-load vs lua-load-per-thread are still reported).

The warning will only be emitted for threads not yet marked as stuck so
as not to interfere with panic dumps and avoid sending a warning just
before a panic. A tainted flag is set when this happens however (0x2000).

(cherry picked from commit 148eb5875fb7e6c46c0a9eac486dcb7b3bca931d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: add a function to dump a stuck thread

There's currently no way to just emit a warning informing that a thread
is stuck without crashing. This is a problem because sometimes users
would benefit from this info to clean up their configuration (e.g. abuse
of map_regm, lua-load etc).

This commit adds a new function ha_stuck_warning() that will emit a
warning indicating that the designated thread has been stuck for XX
milliseconds, with a number of streams blocked, and will make that
thread dump its own state. The warning will then be sent to stderr,
along with some reminders about the impacts of such situations to
encourage users to fix their configuration.

In order not to disrupt operations, a local 4kB buffer is allocated
in the stack. This should be quite sufficient.

For now the function is not used.

(cherry picked from commit 0950778b3a13fe31ff83223827d6692076cba5e5)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: wdt: move the local timers to a struct

Better have a local struct for per-thread timers, as this will allow us
to store extra info that are useful to improve accurate reporting.

(cherry picked from commit 3f4d646849a253f3dc15972e40023495725efe98)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: remove the redundant process.thread_info array from post_mortem

That one is huge and unneeded since we now have the pointer to the
whole thread_info[] array, which does contain the freshest version
of these info and many more. Let's just get rid of it entirely.

(cherry picked from commit 52240680f1d98cc7eb1e762a04becaf54660e96b)
[wt: adjusted ctx in feed_post_mortem_late()]
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: also add fdtab and acitvity to struct post_mortem

These ones are often used as well when trying to analyse sequences of
events, let's add them.

(cherry picked from commit da5cf52173853bcacb12c6ebb045fe395d4b3ba6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: also add a pointer to struct global to post_mortem

The pointer to struct global is also an important element to have in
post_mortem given that it's used a lot to take decisions in the code.
Let's just add it. It's worth noting that we could get rid of argc/argv
at this point since they're also present in the global struct, but they
don't cost much there anyway.

(cherry picked from commit 2f04ebe14aca91f4a0fafcd03a0f310d98d97aaf)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: do not limit backtraces to stuck threads

Historically for size limitation reasons, we would only dump the
backtrace of stuck threads. The problem is that when triggering
a panic or other reasons, we have no backtrace, which effectively
limits it to the watchdog timer. It's also visible in "show threads"
which used to report backtraces for all threads in 2.4 and displays
none nowadays, making its use much more limited.

A first approach could be to just dump the thread that triggers the
panic (in addition to stuck threads). But that remains quite limited
since "show threads" would still display nothing. This patch takes a
better approach consisting in dumping all non-idle threads. This way
the output is less polluted that with the older approach (no need to
dump all those waiting in the poller), and all active threads are
visible, in panics as well as in "show threads". As such, the CLI
command "debug dev panic" now dmups backtraces again. This is already
a benefit which will ease testing of various locations against the
ability to resolve useful symbols.

(cherry picked from commit 4adb2d864d7e3ca9df1e39beabf7b2ffa5aee35c)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: print gdb hints when crashing

To make bug reporting easier for users, when crashing, let's suggest
what to do. Typically when a BUG_ON() matches, only the current thread
is useful the vast majority of the time, while when the watchdog
triggers, all threads are interesting.

The messages are printed at the end after the dump. We may adjust these
with wiki links in the future is more detailed instructions are relevant.

(cherry picked from commit 8f204fa8aeadef3faea4471ba9cfd93d9d168960)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: connection: add new sample fetch functions fc_err_name and bc_err_name

These functions return a symbolic error code such as ECONNRESET to keep
logs compact while making them human-readable. It's a good alternative
to the numeric code in that it's more expressive, and a good one to the
full message since it's shorter and more precise (some codes even match
errno names).

The doc was updated so that the symbolic names appear in the table. It
could be useful to backport this feature to help with troubleshooting
some issues, though backporting the doc might possibly be more annoying
in case users have local patches already, so maybe the table update does
not need to be backported in this case.

(cherry picked from commit 601b34fe7bd50c733a437f26817580bbd56c8d56)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: rawsock: set connection error codes when returning from recv/send/splice

For a long time the errno values returned by recv/send/splice() were not
translated to connection error codes. There are not that many eligible
and having them would help a lot when debugging some complex issues where
logs disagree with network traces. Let's add them now.

(cherry picked from commit 822d82caf4165f0f6da681737c7e3db17d01f599)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: connection: add more connection error codes to cover common errno

While we get reports of connection setup errors in fc_err/bc_err, we
don't have the equivalent for the recv/send/splice syscalls. Let's
add provisions for new codes that cover the common errno values that
recv/send/splice can return, i.e. ECONNREFUSED, ENOMEM, EBADF, EFAULT,
EINVAL, ENOTCONN, ENOTSOCK, ENOBUFS, EPIPE. We also add a special case
for when the poller reported the error itself. It's worth noting that
EBADF/EFAULT/EINVAL will generally indicate serious bugs in the code
and should not be reported.

The only thing is that it's quite hard to forcefully (and reliably)
trigger these errors in automated tests as the timing is critical.
Using iptables to manually reset established connections in the
middle of large transfers at least permits to see some ECONNRESET
and/or EPIPE, but the other ones are harder to trigger.

(cherry picked from commit 00c383ff65c6378327382d2c055f66efb098498d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: stats: Fix the name for the total number of streams created

Because of a copy/paste error, CurrStreams was reused by mistake. It should
be "CumStreams"

No backports needed.

(cherry picked from commit 131b877565db423930909f0c26f25e000cbd6e3b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: stream/stats: Expose the total number of streams ever created in stats

A shared counter is added in the thread context to track the total number of
streams created on the thread. This number is then reported in stats. It
will be a useful information to diagnose some bugs.

(cherry picked from commit 273d322b6fa8117423bbdc9b818002563d4fd3a3)
[wt: ctx adj in tinfo-t]
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: stream/stats: Expose the current number of streams in stats

A shared counter is added in the thread context to track the current number
of streams. This number is then reported in stats. It will be a useful
information to diagnose some bugs.

(cherry picked from commit 18ee22ff766bd7399947af3be2b512ac5827b3c8)
[wt: adj ctx in tinfo-t]
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: cli/debug: show dev: add cmdline and version

'show dev' command is very convenient to obtain haproxy debugging information,
while process is run in container. Let's extend its output with version and
cmdline. cmdline is useful in a way, as it shows absolute binary path and its
arguments, because sometimes the person, who is debugging failing container is
not the same, who has created and deployed it.

argc and argv are stored in the exported global structure, because
feed_post_mortem() is added as a post check function callback in the
post_check_list. So we can't simply change the signature of
feed_post_mortem(), without breaking other post check callbacks APIs.

Parsers are not supposed to modify argv, so we can safely bypass its pointer
to debug_parse_cli_show_dev(), without copying all argument stings somewhere
in the heap or on stack.

(cherry picked from commit 0d79c9bedfa564e3c032c1e910c29949f5133d91)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: quic: fix malformed probing packet building

This bug arrived with this commit:

   cdfceb10a MINOR: quic: refactor qc_prep_pkts() loop

which prevents haproxy from sending PING only packets/datagrams (some
packets/datagrams with only PING frame as ack-eliciting frames inside).
Such packets/datagrams are useful in rare cases during retransmissions
when one wants to probe the peer without exceeding the anti-amplification
limit.

Modify the condition passed to qc_build_pkt() to add padding to the current
datagram. One does not want to do that when probing the peer without ack-eliciting
frames passed as <frms> parameter. Indeed qc_build_pkt() calls qc_do_build_pkt()
which supports this case: if <probe> is true (probing required), qc_do_build_pkt()
handles the case where some padding must be added to a PING only packet/datagram.
This is the case when probing with an empty <frms> frame list of ack-eliciting
frames without exceeding the anti-amplification limit from qc_dgrams_retransmit().

Add some comments to qc_build_pkt() and qc_do_build_pkt() to clarify this
as this code is easy to break!

Thank you for @Tristan971 for having reported this issue in GH #2709.

Must be backported to 3.0.

(cherry picked from commit 217e467e89d15f3c22e11fe144458afbf718c8a8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: connection: properly name the CO_ER_SSL_FATAL enum entry

It was the only one prefixed with "CO_ERR_", making it harder to batch
process and to look up. It was added in 2.5 by commit 61944f7a73 ("MINOR:
ssl: Set connection error code in case of SSL read or write fatal failure")
so it can be backported as far as 2.6 if needed to help integrate other
patches.

(cherry picked from commit 393957908bf492ff6660fba239106f0da7988fe8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: document connection error 44 (reverse connect failure)

It was missing from commit ac1164de7c ("MINOR: connection: define error
for reverse connect"), and can be backported to 3.0 and 2.9.

(cherry picked from commit abed9e0426c2f24522e0053452435082870e3afc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: promex: Fix dump of extra counters

When extra counters are dumped for an entity (frontend, backend, server or
listener), there is a filter on capabilities. Some extra counters are not
available for all entities and must be ignored. However, when this was
performed, the field number, used as an index to dump the metric value, was
still incremented while it should not and leads to an overflow or a stats
mix-up.

This patch must be backported to 3.0.

(cherry picked from commit d1adfd9fe41b0f9f67944eec07348213a7debbf3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: stream: Save last evaluated rule on invalid yield

When an action yields while it is not allowed, an internal error is
reported. This interrupts the processing. So info about the last evaluated
rule must be filled.

This patch may be bakcported if needed. If so, the commit ("MINOR: stream:
Save last evaluated rule on invalid yield") must be backported first.

(cherry picked from commit 0b7605491e4ccb66a0468c219306adf354355e0d)
[cf: Of course the mentionned commit to be backported with this one is wrong. It
     must be "BUG/MINOR: http-ana: Report internal error if an action yields on
     a final eval"].
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: http-ana: Report internal error if an action yields on a final eval

This was already performed for tcp actions at content level, but not for
HTTP actions. It is always a bug, so it must be reported accordingly.

This patch may be backported to all stable versions.

(cherry picked from commit 65ea29dcf85c6553e6dd0613a9c6c506fe22b9ac)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-h1: Fix how timeouts are applied on H1 connections

There were several flaws in the way the different timeouts were applied on
H1 connections. First, the H1C task handling timeouts was not created if no
client/server timeout was specified. But there are other timeouts to
consider. First, the client-fin/server-fin timeouts. But for frontend
connections, http-keey-alive and http-request timeouts may also be used. And
finally, on soft-stop, the close-spread-time value must be considered too.

So at the end, it is probably easier to always create a task to manage H1C
timeouts. Especially since the client/server timeouts are most often set.

Then, when the expiration date of the H1C's task must only be updated if the
considered timeout is set. So tick_add_ifset() must be used instead of
tick_add(). Otherwise, if a timeout is undefined, the taks may expire
immediately while it should in fact never expire.

Finally, the idle expiration date must only be considered for idle
connections.

This patch should be backported in all stable versions, at least as far as
2.6. On the 2.4, it will have to be slightly adapted for the idle_exp
part. On 2.2 and 2.0, the patch will have to be rewrite because
h1_refresh_timeout() is quite different.

(cherry picked from commit 3c09b34325a073e2c110e046f9705b2fddfa91c5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: add missing glitch_{cnt,rate} sample definitions

Following previous commit, when glitch_cnt and glitch_rate data types were
implemented in c9c6b683f ("MEDIUM: stick-tables: add a new stored type for
glitch_cnt and glitch_rate"), newly exposed samples such as
table_glitch_cnt(), table_glitch_rate, src_glitch_cnt() and
src_glitch_rate() were documented but their definitions was missing in
supported keywords list.

It should be backported in 3.0 with c9c6b683f

(cherry picked from commit 0686fd8cfccd7ff12211b8253bf2446d62c90a18)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: add missing glitch_{cnt,rate} data types

When glitch_cnt and glitch_rate data types were implemented in
c9c6b683f ("MEDIUM: stick-tables: add a new stored type for glitch_cnt and
glitch_rate"), the data types list for "stick-table" keyword documentation
was overlooked.

This was reported by Nick Ramirez.

It should be backported in 3.0 with c9c6b683f.

(cherry picked from commit 9a6fc2d474511ead2fe8c39524d23b156d640ef8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: ssl/cli: 'set ssl cert' does not check the transaction name correctly

Since commit  089c13850f ("MEDIUM: ssl: ssl-load-extra-del-ext work
only with .crt"), the 'set ssl cert' CLI command does not check
correctly if the transaction you are trying to update is the right one.

The consequence is that you could commit accidentaly a transaction on
the wrong certificate.

The fix introduces the check again in case you are not using
ssl-load-extra-del-ext.

This must be backported in all stable versions.

(cherry picked from commit 984d2cfb61744bed29ce92cdc5360155cbd8ca44)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: trace: stop rewriting argv with -dt

When using trace with -dt, the trace_parse_cmd() function is doing a
strtok which write \0 into the argv string.

When using the mworker mode, and reloading, argv was modified and the
trace won't work anymore because the first : is replaced by a '\0'.

This patch fixes the issue by allocating a temporary string so we don't
modify the source string directly. It also replace strtok by its
reentrant version strtok_r.

Must be backported as far as 2.9.

(cherry picked from commit 596db3ef86844617565a0b4b4ce8358fe6537d87)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: cli: remove non-printable characters from 'debug dev fd'

When using 'debug dev fd', the output of laddr and raddr can contain
some garbage.

This patch replaces any control or non-printable character by a '.'.

(cherry picked from commit 944a224358ab2865a3a1c0bf700aba38550b19cc)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: store important pointers in post_mortem

Dealing with a core and a stripped executable is a pain when it comes
to finding pools, proxies or thread contexts. Let's put a pointer to
these heads and arrays in the post_mortem struct for easier location.
Other critical lists like this could possibly benefit from being added
later.

Here we now have:
  - tgroup_info
  - thread_info
  - tgroup_ctx
  - thread_ctx
  - pools
  - proxies

Example:
  $ objdump -h haproxy|grep post
   34 _post_mortem  000014b0  0000000000cfd400  0000000000cfd400  008fc400  2**8

  (gdb) set $pm=(struct post_mortem*)0x0000000000cfd400

  (gdb) p $pm->tgroup_ctx[0]
  $8 = {
    threads_harmless = 254,
    threads_idle = 254,
    stopping_threads = 0,
    timers = {
      b = {0x0, 0x0}
    },
    niced_tasks = 0,
    __pad = 0xf5662c <ha_tgroup_ctx+44> "",
    __end = 0xf56640 <ha_tgroup_ctx+64> ""
  }

  (gdb) info thr
    Id   Target Id                         Frame
  * 1    Thread 0x7f9e7706a440 (LWP 21169) 0x00007f9e76a9c868 in raise () from /lib64/libc.so.6
    2    Thread 0x7f9e76a60640 (LWP 21175) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    3    Thread 0x7f9e7613d640 (LWP 21176) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    4    Thread 0x7f9e7493a640 (LWP 21179) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    5    Thread 0x7f9e7593c640 (LWP 21177) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    6    Thread 0x7f9e7513b640 (LWP 21178) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    7    Thread 0x7f9e6ffff640 (LWP 21180) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    8    Thread 0x7f9e6f7fe640 (LWP 21181) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
  (gdb) p/x $pm->thread_info[0].pth_id
  $12 = 0x7f9e7706a440
  (gdb) p/x $pm->thread_info[1].pth_id
  $13 = 0x7f9e76a60640

  (gdb) set $px = *$pm->proxies
  while ($px != 0)
     printf "%#lx %s served=%u\n", $px, $px->id, $px->served
     set $px = ($px)->next
  end

  0x125eda0 GLOBAL served=0
  0x12645b0 stats served=0
  0x1266940 comp served=0
  0x1268e10 comp_bck served=0
  0x1260cf0 <OCSP-UPDATE> served=0
  0x12714c0 <HTTPCLIENT> served=0

(cherry picked from commit e5fccfe0b6397ec2b14ebc3a0d09646442b2018d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: place the post_mortem struct in its own section.

Placing it in its own section will ease its finding, particularly in
gdb which is too dumb to find anything in memory. Now it will be
sufficient to issue this:

  $ gdb -ex "info files" -ex "quit" ./haproxy core 2>/dev/null |grep _post_mortem
  0x0000000000cfd300 - 0x0000000000cfe780 is _post_mortem

or this:

   $ objdump -h haproxy|grep post
    34 _post_mortem  00001480  0000000000cfd300  0000000000cfd300  008fc300  2**8

to spot the symbol's address. Then it can be read this way:

   (gdb) p *(struct post_mortem *)0x0000000000cfd300

(cherry picked from commit 93c3f2a0b4da77c0317496b8585192fb64ef400f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: place a magic pattern at the beginning of post_mortem

In order to ease finding of the post_mortem struct in core dumps, let's
make it start with a recognizable pattern of exactly 32 chars (to
preserve alignment):

  "POST-MORTEM STARTS HERE+7654321\0"

It can then be found like this from gdb:

  (gdb) find 0x000000012345678, 0x0000000100000000, 'P','O','S','T','-','M','O','R','T','E','M'
  0xcfd300 <post_mortem>
  1 pattern found.

Or easier with any other more practical tool (who as ever used "find" in
gdb, given that it cannot iterate over maps and is 100% useless?).

(cherry picked from commit 989b02e1930d7ecd1a728c3d18ccfba095cdd636)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: pools: export the pools variable

We want it to be accessible from debuggers for inspection and it's
currently unavailable. Let's start by exporting it as a first step.

(cherry picked from commit fba48e1c40287f1abb4066935f2436bd0b8cd7a4)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUILD: debug: silence a build warning with threads disabled

Commit 091de0f9b2 ("MINOR: debug: slightly change the thread_dump_pointer
signification") caused the following warning to be emitted when threads
are disabled:

  src/debug.c: In function 'ha_thread_dump_one':
  src/debug.c:359:9: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]

Let's just disguise the pointer to silence it. It should be backported
where the patch above was backported, since it was part of a series aiming
at making thread dumps more exploitable from core dumps.

(cherry picked from commit f163cbfb7f893a06d158880a753cad01908143d8)
[wt: s/MT_LIST_FOR_EACH_ENTRY_LOCKED/mt_list_for_each_entry_safe/ with
     two backup elements in 3.0]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: server: fix race on servers_list during server deletion

Each server is inserted in a global list named servers_list on
new_server(). This list is then only used to finalize servers
initialization after parsing.

On dynamic server creation, there is no issue as new_server() is under
thread isolation. However, when a server is deleted after its refcount
reached zero, srv_drop() removes it from servers_list without lock
protection. In the longterm, this can cause list corruption and crashes,
especially if multiple adjacent servers are removed in parallel.

To fix this, convert servers_list to a mt_list. This should not impact
performance as servers_list is not used during runtime outside of server
creation/deletion.

This should fix github issue #2733. Thanks to Chris Staite who first
found the issue here.

This must be backported up to 2.6.

(cherry picked from commit 7a02fcaf20dbc19db36052bbc7001bcea3912ab5)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: stconn: Don't disable 0-copy FF if EOS was reported on consumer side

There is no reason to disable the 0-copy data forwarding if an end-of-stream
was reported on the consumer side. Indeed, the consumer will send data in
this case. So there is no reason to check the read side here.

This patch may be backported as far as 2.9.

(cherry picked from commit 362de90f3e4ddd0c15331c6b9cb48b671a6e2385)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: http-ana: Fix wrong client abort reports during responses forwarding

When the response forwarding is aborted, we must not report a client abort
if a EOS was seen on client side. On abort performed by the stream must be
considered.

This bug was introduced when the SHUTR was splitted in 2 flags.

This patch must be backported as far as 2.8.

(cherry picked from commit 5970c6abec3e0ee4ac44364e999cae2cc852f4c8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stconn: Report blocked send if sends are blocked by an error

When some data must be sent to the endpoint but an error was previously
reported, nothing is performed and we leave. But, in this case, the SC is not
notified the sends are blocked.

It is indeed an issue if the endpoint reports an error after consuming all
data from the SC. In the endpoint the outgoing data are trashed because of
the error, but on the SC, everything was sent, even if an error was also
reported.

Because of this bug, it is possible to have outgoing data blocked at the SC
level but without any write timeout armed. In some cases, this may lead to
blocking conditions where the stream is never closed.

So now, when outgoing data cannot be sent because an previous error was
triggered, a blocked send is reported. This way, it is possible to report a
write timeout.

This patch should fix the issue #2754. It must be backported as far as 2.8.

(cherry picked from commit fbc3de6e9e59679d2e9ece3984ce31b6a7dd418f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: server: fix dynamic server leak with check on failed init

If a dynamic server is added with check or agent-check, its refcount is
incremented after server keyword parsing. However, if add server fails
at a later stage, refcount is only decremented once, which prevented the
server to be fully released.

This causes a leak with a server which is detached from most of the
lists but still exits in the system.

This bug is considered minor as only a few conditions may cause a
failure in add server after check/agent-check initialization. This is
the case if there is a naming collision or the dynamic ID cannot be
generated.

To fix this, simply decrement server refcount on add server error path
if either check and/or agent-check are flagged as activated.

This bug is related to github issue #2733. Thanks to Chris Staite who
first found the leak.

This must be backported up to 2.6.

(cherry picked from commit 116178563c2fb57e28a76838cf85c4858b185b76)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: activity/memprofile: show per-DSO stats

On systems where many libs are loaded, it's hard to track suspected
leaks. Having a per-DSO summary makes it more convenient. That's what
we're doing here by summarizing all calls per DSO before showing the
total.

(cherry picked from commit 401fb0e87a2cea7171e4d37da6094755eb10a972)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: activity/memprofile: always return "other" bin on NULL return address

It was found in a large "show profiling memory" output that a few entries
have a NULL return address, which causes confusion because this address
will be reused by the next new allocation caller, possibly resulting in
inconsistencies such as "free() ... pool=trash" which makes no sense. The
cause is in fact that the first caller had an entry->info pointing to the
trash pool from a p_alloc/p_free with a NULL return address, and the second
had a different type and reused that entry.

Let's make sure undecodable stacks causing an apparent NULL return address
all lead to the "other" bin.

While this is not exactly a bug, it would make sense to backport it to the
recent branches where the feature is used (probably at least as far as 2.8).

(cherry picked from commit 5091f90479ab4d963b55cb725cee8201d93521d9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: connection/http-reuse: fix address collision on unhandled address families

As described in GH #2765, there were situations where http connections
would be re-used for requests to different endpoints, which is obviously
unexpected. In GH #2765, this occured with httpclient and UNIX socket
combination, but later code analysis revealed that while disabling http
reuse on httpclient proxy helped, it didn't fix the underlying issue since
it was found that conn_calculate_hash_sockaddr() didn't take into account
families such as AF_UNIX or AF_CUST_SOCKPAIR, and because of that the
sock_addr part of the connection wasn't hashed.

To properly fix the issue, let's explicly handle UNIX (both regular and
ABNS) and AF_CUST_SOCKPAIR families, so that the destination address is
properly hashed. To prevent this bug from re-appearing: when the family
isn't known, instead of doing nothing like before, let's fall back to a
generic (unoptimal) hashing which hashes the whole sockaddr_storage struct

As a workaround, http-reuse may be disabled on impacted proxies.
(unfortunately this doesn't help for httpclient since reuse policy
defaults to safe and cannot be modified from the config)

It should be backported to all stable versions.

Shout out to @christopherhibbert for having reported the issue and
provided a trivial reproducer.

[ada: prior to 3.0, ctx adjt is required because conn_hash_update()'s
 prototype is slightly different]

(cherry picked from commit b5b40a9843e505ed84153327ab897ca0e8d9a571)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-h2: Remove H2S from send list if data are sent via 0-copy FF

When data are sent via the zero-copy data forwarding, in h2_done_ff, we must
be sure to remove the H2 stream from the send list if something is send. It
was only performed if no blocking condition was encountered. But we must
also do it if something is sent. Otherwise the transfer may be blocked till
timeout.

This patch must be backported as far as 2.9.

(cherry picked from commit ded28f6e5c210b49ede7edb25cd4b39163759366)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stats-html: Never dump more data than expected during 0-copy FF

During the zero-copy data forwarding, the caller specify the maximum amount
of data the producer may push. However, the HTML stats applet does not use
it and can fill all the free space in the buffer.  It is especially an issue
when the consumer is limited by a flow control, like the H2. Because we may
emit too large DATA frame in this case. It is especially visible with big
buffer (for instance 32k).

In the early age or zero-copy data forwarding, the caller was responsible to
pass a properly resized buffer. And during the different refactoring steps,
this has changed but the HTML stats applet was not updated accordingly.

To fix the bug, the buffer used to dump the HTML page is resized to be sure
not too much data are dumped.

This patch should solve the issue #2757. It must be backported to 3.0.

(cherry picked from commit 529e4f36a353bca292196e1344a79b8cd4ba143c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mux-quic: do not close STREAM with empty FIN if no data sent

A stream may be shut without any HTX EOM reported to report a proper
closure. This is the case for QCS instances flagged with
QC_SF_UNKNOWN_PL_LENGTH. Shut is performed with an empty FIN emission
instead of a RESET_STREAM. This has been implemented since the following
patch :

  24962dd1784dd22babc8da09a5fc8769617f89e3
  BUG/MEDIUM: mux-quic: do not emit RESET_STREAM for unknown length

However, in case of HTTP/3, an empty FIN should only be done after a
full message is emitted, which requires at least a HEADERS frame. If an
empty FIN is emitted without it, client may interpret this as invalid
and close the connection. To prevent this, fallback to a RESET_STREAM
emission if no data were emitted on the stream.

This was reproduced using ngtcp2-client with 10% loss (-r 0.1) on a
remote host, with httpterm request "/?s=100k&C=1&b=0&P=400". An error
ERR_H3_FRAME_UNEXPECTED is returned by ngtcp2-client when the bug
occurs.

Note that this change is incomplete. The message validity depends solely
on the application protocol in use. As such, a new app_ops callback
should be implemented to ensure the stream is closed accordingly.
However, this first patch ensures that at least HTTP/3 case is valid
while keeping a minimal backport process.

This should be backported up to 2.8.

(cherry picked from commit 68c8c910238f0b759d75b4da2128370abf184cd1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mworker: fix mworker-max-reloads parser

Before this patch, when wrong argument was provided in the configuration for
mworker-max-reloads keyword, parser shows these errors below on the stderr:

[WARNING]  (1820317) : config : parsing [haproxy.cfg:154] : (null)parsing [haproxy.cfg:154] : 'mworker-max-reloads' expects an integer argument.

In a case, when by mistake two arguments were provided instead of one, this has
also triggered a buggy error message:

[ALERT]    (1820668) : config : parsing [haproxy.cfg:154] : 'mworker-max-reloads' cannot handle unexpected argument '45'.
[WARNING]  (1820668) : config : parsing [haproxy.cfg:154] : (null)

So, as 'mworker-max-reloads' is parsed in discovery mode by master process
let's align now its parser with all others, which could be called for this
mode. Like this in cases, when there are too many args or argument isn't a
valid integer we return proper error codes to global section parser and
messages are formated properly.

This fix should be backported in all stable versions.

(cherry picked from commit af1d170122369094a1f3869791fb34fb7286e31e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: fix rfc7239 forwarded typo in desc

replace specicy with specify in rfc7239 forwarded option description.
Multiple occurences were found.

May be backported in 2.8.

(cherry picked from commit 45cbbdc84551e51cdaf0046e1371e8495d053fb5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: quic: avoid freezing 0RTT connections

This issue came with this commit:

f627b92 BUG/MEDIUM: quic: always validate sender address on 0-RTT

and could be easily reproduced with picoquic QUIC client with -Q option
which splits a big ClientHello TLS message into two Initial datagrams.
A second condition must be fulfilled to reprodue this issue: picoquic
must not send the token provided by haproxy (NEW_TOKEN). To do that,
haproxy must be patched to prevent it to send such tokens.

Under these conditions, if haproxy has enough time to reply to the first Initial
datagrams, when it receives the second Initial datagram it sends a Retry paquet.
Then the client ignores the Retry paquet as mentionned by RFC 9000:

 17.2.5.2. Handling a Retry Packet
    A client MUST accept and process at most one Retry packet for each connection
    attempt. After the client has received and processed an Initial or Retry packet
    from the server, it MUST discard any subsequent Retry packets that it receives.

On its side, haproxy has closed the connection. When it receives the second
Initial datagram, it open a new connection but with Initial packets it
cannot decrypt (wrong ODCID) leaving the client without response.

To fix this, as the aim of the token (NEW_TOKEN) sent by haproxy is to validate
the peer address, in place of closing the connection when no token was received
for a 0RTT connection, one leaves this validation to the handshake process.
Indeed, the peer adress is validated during the handshake when a valid handshake
packet is received by the listener. But as one does not want haproxy to process
0RTT data when no token was received, one does not accept the connection before
the successful handshake completion. In addition to this, the 0RTT packets
are not released after successful handshake completion when no token was received
to leave a chance to haproxy to process these 0RTT data in such case (see
quic_conn_io_cb()).

Must be backported as far as 2.9.

(cherry picked from commit b1af5dabf0c4af1eda3a520a90332df1f4c12dcf)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: quic: avoid leaking post handshake frames

This bug came with this commit:
f627b92 BUG/MEDIUM: quic: always validate sender address on 0-RTT
If an error happens in quic_build_post_handshake_frames() during the
code exexuted for th NEW_TOKEN frame allocation, some could leak because
of the wrong label used to interrupt this function asap.
Replace the "goto leave" by "goto err" to deallocated such frames to fix
this issue.

Must be backported as far as 2.9.

(cherry picked from commit 19aa320f640f701544c3441787da1577a2479590)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

REGTESTS: Never reuse server connection in http-messaging/truncated.vtc

A "Connection: close" header is added to responses to avoid any connection
reuse. This should avoid errors on the client side.

(cherry picked from commit e7be13da87f8ec00470ef60bb43b85f0480fd85d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MAJOR: filters/htx: Add a flag to state the payload is altered by a filter

When a filter is registered on the data, it means it may change the payload
length by rewritting data. It means consumers of the message cannot trust the
expected length of payload as announced by the producer. The commit 8bd835b2d2
("MEDIUM: filters/htx: Don't rely on HTX extra field if payload is filtered")
was pushed to solve this issue. When the HTTP payload of a message is filtered,
the extra field is set to 0 to be sure it will never be used by error by any
consumer. However, it is not enough.

Indeed, the filters must be called before fowarding some data. They cannot be
by-passed. But if a consumer is unable to flush the HTX message, some outgoing
data can remain blocked in the channel's buffer. If some new data are then
pushed because there is some room in the channel's buffe, the producer will set
the HTX extra field. At this stage, if the consumer is unblocked and can send
again data, it is possible to call it to forward outgoing data blocked in the
channel's buffer before waking the stream up to filter new input data. It is the
purpose of the data fast-forwarding. In this case, the HTX extra field will be
seen by the consumer. It is unexpected and leads to undefined behavior.

One consequence of this bug is to perform a wrong chunking on compressed
messages, leading to processing errors at the end of the message, reported as
"ID--" in logs.

To fix the bug, a HTX flag is added to state the payload of the current HTX
message is altered. When this flag is set (HTX_FL_ALTERED_PAYLOAD), the HTX
extra field must not be trusted. And to keep things simple, when this flag is
set, the HTX extra field is automatically set to 0 when the HTX message is
loaded, in htxbuf() function.

It is probably the less intrusive way to fix the bug for now. But this part must
be reviewed to save meta-info of the HTX message outside of the message itself.

This commit should solve the issue #2741. It must be backported as far as 2.9.

(cherry picked from commit 52a3d807fc332b57b62f5e30aa6f697636a22695)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stconn: Check FF data of SC to perform a shutdown in sc_notify()

In sc_notify() function, the consumer side of the SC is tested to verify if
we must perform a shutdown on the endpoint. To do so, no output data must be
present in the buffer and in the iobuf. However, there is a bug here, the
iobuf of the opposite SC is tested instead of the one of the current SC. So
a shutdown can be performed on the endpoint while there are still output
data in the iobuf that must be sent. Concretely, it can only be data blocked
in a pipe.

Because of this bug, data blocked in the pipe will be never sent. I've not
tested but I guess this may block the stream during the client or server
timeout.

This patch must be backported as far as 2.9.

(cherry picked from commit 0fcfed9e231f2bc3963fe6085598970db2174af1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: http-ana: Don't report a server abort if response payload is invalid

If a parsing error is reported by the mux on the response payload, a proxy
error (PRXCOND) must be reported instead of a server abort (SRVCL). Because
of this bug, inavlid response may are reported as "SD--" or "SL--" in logs
instead of "PD--" or "PL--".

This patch must be backported to all stable versions.

(cherry picked from commit 6790067e79566b2ca5943e72200361c40001bde2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stconn: Wait iobuf is empty to shut SE down during a check send

When a send attempt is performed on the opposite side from sc_notify() and
all outgoing data are sent while a shut was scheduled, the SE is shut down
because we consider all data were sent and no more are expected. However,
here we must also be carefull to have sent all pending data in the
iobuf. Indeed, some spliced data may be blocked. In this case, if the SE is
shut down, these data may be lost.

This patch should fix the original bug reported in #2749. It must be
backported as far as 2.9.

(cherry picked from commit 48f1e2b6fe8457bb5b9d8db9447157c244d871b7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: httpclient: return NULL when no proxy available during httpclient_new()

Latest patches on the mworker rework skipped the httpclient_proxy
creation by accident. This is not supposed to happen because haproxy is
supposed to stop when the proxy creation failed, but it shows a flaw in
the API.

When the httpclient_proxy or the proxy used in parameter of
httpclient_new_from_proxy() is NULL, it will be dereferenced and cause a
crash.

The patch only returns a NULL when doing an httpclient_new() if the
proxy is not available.

Must be backported as far as 2.7.

(cherry picked from commit e7b7072943d658702eba3651d66c6093f1a79fa8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: queue: make sure never to queue when there's no more served conns

Since commit 53f52e67a0 ("BUG/MEDIUM: queue: always dequeue the backend when
redistributing the last server"), we've got two reports again still showing
the theoretically impossible condition in pendconn_add(), including a single
threaded one.

Thanks to the traces, the issue could be tracked down to the redispatch part.
In fact, in non-determinist LB algorithms (RR, LC, FAS), we don't perform the
LB if there are pending connections in the backend, since it indicates that
previous attempts already failed, so we directly return SRV_STATUS_FULL. And
contrary to a previous belief, it is possible to meet this condition with
be->served==0 when redispatching (and likely with maxconn not greater than
the number of threads).

The problem is that in this case, the entry is queued and then the
pendconn_must_try_again() function checks if any connections are currently
being served to detect if we missed a race, and tries again, but that
situation is not caused by a concurrent thread and will never fix itself,
resulting in the loop.

All that part around pendconn_must_try_again() is still quite brittle, and
a safer approach would involve a sequence counter to detect new arrivals
and dequeues during the pendconn_add() call. But it's more sensitive work,
probably for a later fix.

This fix must be backported wherever the fix above was backported. Thanks
to Patrick Hemmer, as well as Damien Claisse and Basha Mougamadou from
Criteo for their help on tracking this one!

(cherry picked from commit ca275d99ce02e72d707fc87da133d739cdda5146)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-quic: ensure timeout server is active for short requests

If a small request is received on QUIC MUX frontend, it can be
transmitted directly with the FIN on attach operation. rcv_buf is
skipped by the stream layer. Thus, it is necessary to ensure that there
is similar behavior when FIN is reported either on attach or rcv_buf.

One difference was that se_expect_data() was called only for rcv_buf but
not on attach. This most obvious effect is that stream timeout was
deactivated for this request : client timeout was disabled on EOI but
server one not armed due to previous se_expect_no_data(). This prevents
the early closure of too long requests.

To fix this, add an invokation of se_expect_data() on attach operation.

This bug can simply be detected using httpterm with delay request (for
example /?t=10000) and using smaller client/server timeouts. The bug is
present if the request is not aborted on timeout but instead continue
until its proper HTTP 200 termination.

This has been introduced by the following commit :
  85eabfbf672c57e4ed082da1b96c95348b331320
  MEDIUM: mux-quic: Don't expect data from server as long as request is unfinished

This must be backported up to 2.8.

(cherry picked from commit 232083c3e5ca3f23a44fa64def6a88dd257c3b23)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: hlua: properly handle sample func errors in hlua_run_sample_{fetch,conv}()

To execute sample fetches and converters from lua. hlua API leverages the
sample API. Prior to executing the sample func, the arg checker is called
from hlua_run_sample_{fetch,conv}() to detect potential errors.

However, hlua_run_sample_{fetch,conv}() both pass NULL as <err> argument,
but it is wrong for two reasons. First we miss an opportunity to report
precise error messages to help the user know what went wrong during the
check.. and more importantly, some val check functions consider that the
<err> pointer is never NULL. This is the case for example with
check_crypto_hmac(). Because of this, when such val check functions
encounter an error, they will crash the process because they will try
to de-reference NULL.

This bug was discovered and reported by GH user @JB0925 on #2745.

Perhaps val check functions should make sure that the provided <err>
pointer is != NULL prior to de-referencing it. But since there are
multiple occurences found in the code and the API isn't clear about that,
it is easier to fix the hlua part (caller) for now.

To fix the issue, let's always provide a valid <err> pointer when
leveraging val_arg() check function pointer, and make use of it in case
or error to report relevant message to the user before freeing it.

It should be backported to all stable versions.

(cherry picked from commit f88f162868df9053ca71e3be0628221c36153d9a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: hlua: make hlua_ctx_renew() safe

hlua_ctx_renew() is called from unsafe places where the caller doesn't
expect it to LJMP.. however hlua_ctx_renew() makes use of Lua library
function that could potentially raise errors, such as lua_newthread(),
and it does nothing to catch errors. Because of this, haproxy could
unexpectedly crash. This was discovered and reported by GH user
@JB0925 on #2745.

To fix the issue, let's simply make hlua_ctx_renew() safe by applying
the same logic implemented for hlua_ctx_init() or hlua_ctx_destroy(),
which is catching Lua errors by leveraging SET_SAFE_LJMP_PARENT() helper.

It should be backported to all stable versions.

(cherry picked from commit d0e01051813bde5cb06bebe88102f2b2885b3dea)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: server: server stuck in maintenance after FQDN change

Pierre Bonnat reported that SRV-based server-template recently stopped
to work properly.

After reviewing the changes, it was found that the regression was caused
by a4d04c6 ("BUG/MINOR: server: make sure the HMAINT state is part of MAINT")

Indeed, HMAINT is not a regular maintenance flag. It was implemented in
b418c122 a4d04c6 ("BUG/MINOR: server: make sure the HMAINT state is part
of MAINT"). This flag is only set (and never removed) when the server FQDN
is changed from its initial config-time value. This can happen with "set
server fqdn" command as well as SRV records updates from the DNS. This
flag should ideally belong to server flags.. but it was stored under
srv_admin enum because cur_admin is properly exported/imported via server
state-file while regular server's flags are not.

Due to a4d04c6, when a server FQDN changes, the server is considered in
maintenance, and since the HMAINT flag is never removed, the server is
stuck in maintenance.

To fix the issue, we partially revert a4d04c6. But this latter commit is
right on one point: HMAINT flag was way too confusing and mixed-up between
regular MAINT flags, thus there's nothing to blame about a4d04c6 as it was
error-prone anyway.. To prevent such kind of bugs from happening again,
let's rename HMAINT to something more explicit (SRV_ADMF_FQDN_CHANGED) and
make it stand out under srv_admin enum so we're not tempted to mix it with
regular maintenance flags anymore.

Since a4d04c6 was set to be backported in all versions, this patch must
be backported there as well.

(cherry picked from commit 85298189bf4c268b15c33aea95e0cc35113e25f0)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

MEDIUM: debug: on panic, make the target thread automatically allocate its buf

One main problem with panic dumps is that they're filling the dumping
thread's trash, and that the global thread_dump_buffer is too small to
catch enough of them.

Here we're proceeding differently. When dumping threads for a panic, we're
passing the magic value 0x2 as the buffer, and it will instruct the target
thread to allocate its own buffer using get_trash_chunk() (which is signal
safe), so that each thread dumps into its own buffer. Then the thread will
wait for the buffer to be consumed, and will assign its own thread_dump_buffer
to it. This way we can simply dump all threads' buffers from gdb like this:

  (gdb) set $t=0
        while ($t < global.nbthread)
          printf "%s\n", ha_thread_ctx[$t].thread_dump_buffer.area
          set $t=$t+1
        end

For now we make it wait forever since it's only called on panic and we
want to make sure the thread doesn't leave and continues to use that trash
buffer or do other nasty stuff. That way the dumping thread will make all
of them die.

This would be useful to backport to the most recent branches to help
troubleshooting. It backports well to 2.9, except for some trivial
context in tinfo-t.h for an updated comment. 2.8 and older would also
require TAINTED_PANIC. The following previous patches are required:

   MINOR: debug: make mark_tainted() return the previous value
   MINOR: chunk: drop the global thread_dump_buffer
   MINOR: debug: split ha_thread_dump() in two parts
   MINOR: debug: slightly change the thread_dump_pointer signification
   MINOR: debug: make ha_thread_dump_done() take the pointer to be used
   MINOR: debug: replace ha_thread_dump() with its two components

(cherry picked from commit 278b9613a32ddb0b5ffa1d66e6c25f434758c65a)
[wt: ctx updt in tinfo-t for comment]
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: replace ha_thread_dump() with its two components

At the few places we were calling ha_thread_dump(), now we're
calling separately ha_thread_dump_fill() and ha_thread_dump_done()
once the data are consumed.

(cherry picked from commit afeac4bc026e64e27d99e50480bff5bc1ee60cb9)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: make ha_thread_dump_done() take the pointer to be used

This will allow the caller to decide whether to definitely clear the
pointer and release the thread, or to leave it unlocked so that it's
easy to analyse from the struct (the goal will be to use that in panic()
so that cores are easy to analyse).

(cherry picked from commit d7c34ba479f79ed42c709c64679b4e3d1310cddd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: slightly change the thread_dump_pointer signification

Now the thread_dump_pointer is returned ORed with 1 once done, or NULL
when cancelled (for now noone cancels). The goal will be to permit
the callee to provide its own pointer.

The ha_thread_dump_fill() function now returns the buffer pointer that
was used (without OR 1) or NULL, for ease of use from the caller.

(cherry picked from commit 091de0f9b2553463660a11c56598c4970d6b1066)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: split ha_thread_dump() in two parts

We want to have a function to trigger the dump and another one to wait
for it to be completed. This will be important to permit panic dumps to
be done on local threads. For now this does not change anything, as the
function still calls the two new functions one after the other.

(cherry picked from commit 2036f5bba1637c8dad3eca577aa3bed4cc53caaf)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: chunk: drop the global thread_dump_buffer

This variable is not very useful and is confusing anyway. It was mostly
used to detect that a panic dump was still in progress, but we can now
check mark_tainted() for this. The pointer was set to one of the dumping
thread's trash chunks. Let's temporarily continue to copy the dumps to
that trash, we'll remove it later.

(cherry picked from commit a6698304e03847f3e114b22b4229b382f3ddffd1)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: debug: make mark_tainted() return the previous value

Since mark_tainted() uses atomic ops to update the tainted status, let's
make it return the prior value, which will allow the caller to detect
if it's the first one to set it or not.

(cherry picked from commit 8e048603d1bba0721433a5ae0480fbf1ab6f4897)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: http-ana: Disable fast-fwd for unfinished req waiting for upgrade

If a request is waiting for a protocol upgrade but it is not finished, the
data fast-forwarding is disabled. Otherwise, the request analyzers will miss
the end of the message.

This case is possible since the commit 01fb1a54 ("BUG/MEDIUM: mux-h1/mux-h2:
Reject upgrades with payload on H2 side only"). Indeed, before, a protocol
upgrade was not allowed for request with payload. But it is now possible and
this comes with a side-effect. It is not really satisfying but for now there
is no other way to sync the muxes and the applicative stream. It seems to be
a reasonnable fix for now, waiting for a deeper refactoring.

This patch must be backported with the commit above.

(cherry picked from commit cea1379cf1fcd5fd9c3b3b104f4c5374bc44bce2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mux-h1: Fix condition to set EOI on SE during zero-copy forwarding

During zero-copy data forwarding, the producer must set the EOI flag on the SE
when end of the message is reached. It is already done but there is a case where
this flag is set while it should not. When a request wants to perform a protocol
upgrade and it is waiting for the server response, the flag must not be set
because the HTTP message is finished but some data are possibly still expected,
depending on the server response. On a 101-switching-protocol, more data will be
sent because the producer is switch to TUNNEL state.

So, now, the right condition is used. In DONE state, SE_FL_EOI flag is set on the sedesc iff:

  - it is the response
  - it is the request and the response is also in DONNE state
  - it is a request but no a protocol upgrade nor a CONNECT

This patch must be backported as far as 2.9.

(cherry picked from commit 6b39e245e1d58698310fbb06b8122423232be9a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: queue: always dequeue the backend when redistributing the last server

An interesting bug was revealed by commit 5541d4995d ("BUG/MEDIUM: queue:
deal with a rare TOCTOU in assign_server_and_queue()"). When shutting
down a server to redistribute its connections, no check is made on the
backend's queue. If we're turning off the last server and the backend
has pending connections, these ones will wait there till the queue
timeout. But worse, since the commit above, we can enter an endless loop
in the following situation:

  - streams are present in the backend's queue
  - streams are purged on the last server via srv_shutdown_streams()
  - that one calls pendconn_redistribute(srv) which does not purge
    the backend's pendconns
  - a stream performs some load balancing and enters assign_server_and_queue()
  - assign_server() is called in turn
  - the LB algo is non-deterministic and there are entries in the
    backend's queue. The function notices it and returns SRV_STATUS_FULL
  - assign_server_and_queue() calls pendconn_add() to add the connection
    to the backend's queue
  - on return, pendconn_must_try_again() is called, it figures there's
    no stream served anymore on the server nor the proxy, so it removes
    the pendconn from the queue and returns 1
  - assign_server_and_queue() loops back to the beginning to try again,
    while the conditions have not changed, resulting in an endless loop.

Ideally a change count should be used in the queues so that it's possible
to detect that some dequeuing happened and/or that a last stream has left.
But that wouldn't completely solve the problem that is that we must never
ever add to a queue when there's no server streams to dequeue the new
entries.

The current solution consists in making pendconn_redistribute() take care
of the proxy after the server in case there's no more server available on
the proxy. It at least ensures that no pending streams are left in the
backend's queue when shutting streams down or when the last server goes
down. The try_again loop remains necessary to deal with inevitable races
during pendconn additions. It could be limited to a few rounds, though,
but it should never trigger if the conditions are sufficient to permit
it to converge.

One way to reproduce the issue is to run a config with a single server
with maxconn 1 and plenty of threads, then run in loops series of:

 "disable server px/s;shutdown sessions server px/s;
  wait 100ms server-removable px/s; show servers conn px;
  enable server px/s"

on the CLI at ~10/s while injecting with around 40 concurrent conns at
40-100k RPS. In this case in 10s - 1mn the crash can appear with a
backtrace like this one for at least 1 thread:

  #0  pendconn_add (strm=strm@entry=0x17f2ce0) at src/queue.c:487
  #1  0x000000000064797d in assign_server_and_queue (s=s@entry=0x17f2ce0) at src/backend.c:1064
  #2  0x000000000064a928 in srv_redispatch_connect (s=s@entry=0x17f2ce0) at src/backend.c:1962
  #3  0x000000000064ac54 in back_handle_st_req (s=s@entry=0x17f2ce0) at src/backend.c:2287
  #4  0x00000000005ae1d5 in process_stream (t=t@entry=0x17f4ab0, context=0x17f2ce0, state=<optimized out>) at src/stream.c:2336

It's worth noting that other threads may often appear waiting after the
poller and one in server_atomic_sync() waiting for isolation, because
the event that is processed when shutting the server down is consumed
under isolation, and having less threads available to dequeue remaining
requests increases the probability to trigger the problem, though it is
not at all necessary (some less common traces never show them).

This should carefully be backported wherever the commit above was
backported.

(cherry picked from commit 53f52e67a08fc2cd5ae64caf4148a5de884f9959)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: server: make srv_shutdown_sessions() call pendconn_redistribute()

When shutting down server sessions, the queue was not considered, which
is a problem if some element reached the queue at the moment the server
was going down, because there will be no more requests to kick them out
of it. Let's always make sure we scan the queue to kick these streams
out of it and that they can possibly find a more suitable server. This
may make a difference in the time it takes to shut down a server on the
CLI when lots of servers are in the queue.

It might be interesting to backport this to 3.0 but probably not much
further.

(cherry picked from commit 1d403caf8aa59c9070f30ea16017261cab679fe2)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: queue: make sure that maintenance redispatches server queue

Turning a server to maintenance currently doesn't redispatch the server
queue unless there's an explicit "option redispatch" and no "option
persist", while the former has never really been the purpose of this
test. Better refine this so that forced maintenance also causes the
queue to be flushed, and possibly redispatched unless the proxy has
option persist. This way now when turning a server to maintenance,
the queue is immediately flushed and streams can decide what to do.

This can be backported, though there's no need to go far since it was
never directly reported and only noticed as part of debugging some
rare "shutdown sessions" strangeness, which it might participate to.

(cherry picked from commit 1385e33eb089093dbc970dbc2759d2969ae533c5)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: stream: make stream_shutdown() async-safe

The solution found in commit b500e84e24 ("BUG/MINOR: server: shut down
streams under thread isolation") to deal with inter-thread stream
shutdown doesn't work fine because there exists code paths involving
a server lock which can then deadlock on thread_isolate(). A better
solution then consists in deferring the shutdown to the stream itself
and just wake it up for that.

The only thing is that TASK_WOKEN_OTHER is a bit too generic and we
need to pass at least 2 types of events (SF_ERR_DOWN and SF_ERR_KILLED),
so we're now leveraging the new TASK_F_UEVT1 and _UEVT2 flags on the
task's state to convey these info. The caller only needs to wake the
task up with these flags set, and the stream handler will then finish
the job locally using stream_shutdown_self().

This needs to be carefully backported to all branches affected by the
dequeuing issue and containing any of the 5541d4995d ("BUG/MEDIUM:
queue: deal with a rare TOCTOU in assign_server_and_queue()"), and/or
b11495652e ("BUG/MEDIUM: queue: implement a flag to check for the
dequeuing").

(cherry picked from commit b8e3b0a18d59b4f52b4ecb7ae61cef0b8b2402a0)
Signed-off-by: Willy Tarreau <w@1wt.eu>