Commit Graph

23780 Commits

Author SHA1 Message Date
Aurelien DARRAGON
22229a41a2 MEDIUM: stktable: split src-based key smp_fetch_sc functions
In this patch we split several sample fetch functions that are leveraged
by the "src-" fetches such as smp_fetch_sc_inc_gpc().

Indeed, for all of them, we add an intermediate helper function that takes
a stkctr pointer as parameter and performs the logic, leaving the lookup
part in the calling function. Before this patch existing functions were
doing the lookup + the fetch logic. Thanks to this patch it will become
easier to add generic converters taking lookup key as input.

List of targeted functions:
 - smp_fetch_sc_inc_gpc()
 - smp_fetch_sc_inc_gpc0()
 - smp_fetch_sc_inc_gpc1()
 - smp_fetch_sc_clr_gpc()
 - smp_fetch_sc_clr_gpc0()
 - smp_fetch_sc_clr_gpc1()
 - smp_fetch_sc_conn_cnt()
 - smp_fetch_sc_conn_rate()
 - smp_fetch_sc_updt_conn_cnt()
 - smp_fetch_sc_conn_curr()
 - smp_fetch_sc_glitch_cnt()
 - smp_fetch_sc_glitch_rate()
 - smp_fetch_sc_sess_cnt()
 - smp_fetch_sc_sess_rate()
 - smp_fetch_sc_http_req_cnt()
 - smp_fetch_sc_http_req_rate()
 - smp_fetch_sc_http_err_cnt()
 - smp_fetch_sc_http_err_rate()
 - smp_fetch_sc_http_fail_cnt()
 - smp_fetch_sc_http_fail_rate()
 - smp_fetch_sc_kbytes_in()
 - smp_fetch_sc_bytes_in_rate()
 - smp_fetch_kbytes_out()
 - smp_fetch_sc_gpc1_rate()
 - smp_fetch_sc_gpc0_rate()
 - smp_fetch_sc_gpc_rate()
 - smp_fetch_sc_get_gpc1()
 - smp_fetch_sc_get_gpc0()
 - smp_fetch_sc_get_gpc()
 - smp_fetch_sc_get_gpt0()
 - smp_fetch_sc_get_gpt()
 - smp_fetch_sc_bytes_out_rate()

Please note that this patch doesn't render any good using "git show" or
"git diff". For all the functions listed above, a new helper function was
defined right above it, with the same name without "_sc". These new
functions perform the fetch part, while the original ones (with "_sc")
now simply perform the lookup and then leverage the corresponding fetch
helper.
2025-01-15 14:04:45 +01:00
Aurelien DARRAGON
f71bad4694 MINOR: stktable: add smp_fetch_stksess() helper function
smp_fetch_stksess(table, smp, create) performs a lookup in <table> by
using <smp> as a key. It returns matching entry on success and NULL on
failure. <create> can be set to 1 to force the entry creation.

We then use this helper everywhere relevant to prevent code duplication
2025-01-15 14:04:40 +01:00
Aurelien DARRAGON
0fb8807820 MINOR: stktable: fix potential build issue in smp_to_stkey (2nd try)
As discussed in GH #2838, the previous fix f399dbf
("MINOR: stktable: fix potential build issue in smp_to_stkey") which
attempted to remove conversion ambiguity and prevent build warning proved
to be insufficient.

This time, we implement Willy's suggestion, which is to use an union to
perform the conversion.

Hopefully this should fix GH #2838. If that's the case (and only in that
case), then this patch may be backported with f399dbf (else the patch
won't apply) anywhere b59d1fd ("BUG/MINOR: stktable: fix big-endian
compatiblity in smp_to_stkey()") was backported.
2025-01-15 14:04:31 +01:00
Christopher Faulet
91578212d7 BUG/MEDIUM: promex: Use right context pointers to dump backends extra-counters
When backends extra counters are dumped, the wrong pointer was used in the
promex context to retrieve the stats module. p[1] must be used instead of
p[2]. Because of this typo, a infinite loop could be experienced if the
output buffer is full during this stage. But in all cases an overflow is
possible leading to a memory corruption.

This patch may be related to issue #2831. It must be backported as far as
3.0.
2025-01-14 15:38:43 +01:00
Aurelien DARRAGON
8919a80da9 BUG/MEDIUM: stktable: fix missing lock on some table converters
In 819fc6f563
("MEDIUM: threads/stick-tables: handle multithreads on stick tables"),
sample fetch and action functions were properly guarded with stksess
read/write locks for read and write operations respectively, but the
sample_conv_table functions leveraged by "table_*" converters were
overlooked.

This bug was not known to cause issues in existing deployments yet (at
least it was not reported), but due to its nature it can theorically lead
to inconsistent values being reported by "table_*" converters if the value
is being updated by another thread in parallel.

It should be backported to all stable versions.

[ada: for versions < 3.0, glitch_cnt and glitch_rate samples should be
 ignored as they first appeared in 3.0]
2025-01-14 11:36:04 +01:00
Aurelien DARRAGON
f399dbf70c MINOR: stktable: fix potential build issue in smp_to_stkey
smp_to_stkey() uses an ambiguous cast from 64bit integer to 32 bit
unsigned integer. While it is intended, let's make the cast less
ambiguous by explicitly casting the right part of the assignment to the
proper type.

This should fix GH #2838
2025-01-13 09:45:40 +01:00
Amaury Denoyelle
4a5d82a97d BUG/MINOR: quic: reject NEW_TOKEN frames from clients
As specified by RFC 9000, reject NEW_TOKEN frames emitted by clients.
Close the connection with error code PROTOCOL_VIOLATION.

This must be backported up to 2.6.
2025-01-10 14:50:59 +01:00
Amaury Denoyelle
a2c0c459a4 MINOR: trace: support all source alias on -dt
Command line argument -dt can be used to activate traces during startup.
Via its optional argument, it is possible to change settings for a
particular trace source. It is also possible to update every registered
sources by specifying an empty name.

Support the trace source alias "all". This is an alternative to the
empty name to update every sources.
2025-01-10 14:50:59 +01:00
Amaury Denoyelle
a50dd07c16 MINOR: trace: ensure -dt priority over traces config section
Traces can be activated on startup either via -dt command line argument
or via the traces configuration section. This can caused confusion as it
may not be clear as trace source can be completed or overriden by one or
the other.

Fix the precedence to give the priority to the command line argument.
Now, each trace source configured via -dt is first resetted to a default
state before applying new settings. Then, it is impossible to change a
trace source via the configuration file if it was already targetted via
-dt argument.
2025-01-10 14:50:59 +01:00
Amaury Denoyelle
da9a7e0bd9 MINOR: trace: add help message for -dt argument
Traces can be activated on startup via -dt command line argument. To
facilitate its usage, display a usage description and examples when
"help" is specified.
2025-01-10 14:50:59 +01:00
Olivier Houchard
659d5f6579 BUG/MEDIUM: queues: Adjust the proxy counters when appropriate
In process_srv_queue(), if we manage to successfully run an extra task,
don't forget to adjust the proxy's totpend and served counters accordingly.
Having an inaccurate served could lead to various subtle bugs, as it is
used when making load balancing decisions.

This should not be backported, unless cda7275ef5
is backported too.
2025-01-09 17:46:46 +01:00
Aurelien DARRAGON
24042df94e MINOR: stktable: add sc[0-2]_key fetches
As discussed in GH #1750, we were lacking a sample fetch to be able to
retrieve the key from the currently tracked counter entry. To do so,
sc_key fetch can now be used. It returns a sample with the correct type
(table key type) corresponding to the tracked counter entry (from previous
track-sc rules).

If no entry is currently tracked, it returns nothing.

It can be used using the standard form "sc_key(<sc_number>)" or the legacy
form: "sc0_key", "sc1_key", "sc2_key"

Documentation was updated.
2025-01-09 10:57:01 +01:00
Aurelien DARRAGON
7423310d5d MINOR: stktable: add stksess_getkey() helper
stksess_getkey(t, ts) returns a stktable_key struct pointer filled with
data from input <ts> entry in <t> table. Returned pointer uses the
static_table_key variable. Indeed, stktable_key struct is more convenient
to manipulate than having to deal with the key extraction from stktsess
struct directly.
2025-01-09 10:56:56 +01:00
Aurelien DARRAGON
df9c2ef2c3 MINOR: stktable: add stkey_to_smp() helper
reverse operation for smp_to_stkey(): fills input <smp> from a
stktable_key struct.

Returns 1 on success and 0 on failure.
2025-01-09 10:56:50 +01:00
Aurelien DARRAGON
b59d1fd911 BUG/MINOR: stktable: fix big-endian compatiblity in smp_to_stkey()
When smp_to_stkey() deals with SINT samples, since stick-tables deals with
32 bits integers while SINT sample is 64 bit integer, inplace conversion
was done in smp_to_stkey. For that the 64 bit integer was truncated before
the key would point to it. Unfortunately this only works on little endian
architectures because with big endian ones, the key would point to the
wrong 32bit range.

To fix the issue and make the conversion endian-proof, let's re-assign
the sample as 32bit integer before the key points to it.

Thanks to Willy for having spotted the bug and suggesting the above fix.

It should be backported to all stable versions.
2025-01-09 10:56:43 +01:00
Willy Tarreau
7be596b35c [RELEASE] Released version 3.2-dev3
Released version 3.2-dev3 with the following main changes :
    - DOC: config: add missing "track-sc0" in action keywords matrix
    - BUG/MINOR: stktable: invalid use of stkctr_set_entry() with mixed table types
    - BUG/MAJOR: mux-quic: fix BUG_ON on empty STREAM emission
    - BUG/MEDIUM: mux-h2: Count copied data when looping on RX bufs in h2_rcv_buf()
    - Revert "BUG/MAJOR: mux-quic: fix BUG_ON on empty STREAM emission"
    - BUG/MAJOR: mux-quic: properly fix BUG_ON on empty STREAM emission
    - MINOR: mux-quic: add traces on sd attach
    - BUG/MEDIUM: mux-quic: do not attach on already closed stream
    - BUG/MINOR: compression: handle a possible strdup() failure
    - BUG/MINOR: pool: handle a possible strdup() failure
    - BUG/MINOR: cfgparse-tcp: handle a possible strdup() failure
    - BUG/MINOR: log: Allow to use if/unless conditionnals for do-log action
    - MINOR: config: Alert about extra arguments for errorfile and errorloc
    - BUG/MINOR: mux-quic: fix wakeup on qcc_set_error()
    - MINOR: mux-quic: change return value of qcs_attach_sc()
    - BUG/MINOR: mux-quic: handle closure of uni-stream
    - BUG/MEDIUM: promex/resolvers: Don't dump metrics if no nameserver is defined
    - BUG/MAJOR: ssl/ocsp: fix NULL conn object dereferencing to access QUIC TLS counters
    - MEDIUM: errors: get rid of shm_open()
    - BUILD: makefile: do not clean standalone binaries on a simple "make clean"
    - BUILD: makefile: add a qinfo macro to pass info in quiet mode
    - DEV: ncpu: add a simple utility to help with NUMA development
    - DEV: ncpu: implement a wrapper mode
    - DEV: ncpu: make the wrapper work both as a lib and executable
    - BUG/MEDIUM: h1-htx: Properly handle bodyless messages
    - MINOR: tools: add a few functions to simply check for a file's existence
2025-01-09 09:21:04 +01:00
Willy Tarreau
b25850f25b MINOR: tools: add a few functions to simply check for a file's existence
At many places we'd like to be able to simply construct a path from a
format string and check if that path corresponds to an existing file,
directory etc. Here we add 3 functions, a generic one to test that a
path corresponds to a given file mode (e.g. S_IFDIR, S_IFREG etc), and
two other ones specifically checking for a file or a dir for easier
use.
2025-01-09 09:18:49 +01:00
Christopher Faulet
b9cc361b35 BUG/MEDIUM: h1-htx: Properly handle bodyless messages
During h1 parsing, there are some postparsing checks to detect bodyless
messages and switch the parsing in DONE state. However, a case was not
properly handled. Responses to HEAD requests with a "transfer-encoding"
header. The response parser remained blocked waiting for the response body.

To fix the issue, the postparsing was sliglty modified. Instead of trying to
handle bodyless messages in a common way between the request and the
response, it is now performed in the dedicated postparsing functions. It is
easier to enumerate all cases, especially because there is already a test
for responses to HEAD requests.

This patch should fix the issue #2836. It must be backported as far as 2.9.
2025-01-08 18:20:26 +01:00
Willy Tarreau
ca773e1a2a DEV: ncpu: make the wrapper work both as a lib and executable
It's convenient to have a share lib be able to also work as a wrapper.
But recent glibc broke support for this dual-mode thing some time ago:

   https://patchwork.ozlabs.org/project/glibc/patch/20190312130235.8E82C89CE49C@oldenburg2.str.redhat.com/
   https://stackoverflow.com/questions/59074126/loading-executable-or-executing-a-library

Trying to preload such an executable indeed returns:

   ERROR: ld.so: object '/path/to/ncpu.so' from LD_PRELOAD cannot be preloaded (cannot dynamically load position-independent executable): ignored.

Note that the code still supports it since libc.so is both an executable
and a lib. The approach taken here is the same as in the nousr.so wrapper.
It consists in dropping the DF_1_PIE flag from the resulting executable
since it's what the dynamic linker is looking for. This flag is found in
FLAGS_1 in the .dynamic section. As readelf -a suggests, it's after the
tag 0x6ffffffb. The value is 0x08000000. We're using objdump to figure the
length and offset of the struct, dd to extract the 3 parts, and sed to
patch the binary.

It's likely that it will only work on 64-bit little endian, though tests
should be performed to see what to do on other platforms. At least on
x86_64, ld.so is happy and it continues to be possible to use the binary
as a .so, and that the platform where most of the development happens so
that's fine.

In any case the wrapper and the standard shared lib are still made two
distinct files so that it's possible to use the non-patched version on
unsupported OSes or architectures.
2025-01-08 11:27:10 +01:00
Willy Tarreau
3fdf875716 DEV: ncpu: implement a wrapper mode
The wrapper mode allows to present itself as LD_PRELOAD before loading
haproxy, which is often more convenient since it allows to pass the
number of CPUs in argument. However, this mode is no longer supported by
modern glibcs, so a future patch will come to implement a trick that was
tested to work at least on x86.
2025-01-08 11:26:05 +01:00
Willy Tarreau
25c08562cb DEV: ncpu: add a simple utility to help with NUMA development
Collecting captures of /sys isn't sufficient for NUMA development because
haproxy detects the number of CPUs at boot time and will not be able to
inspect more than this number. Let's just have a small utility to report
a fake number of CPUs, that will be loaded using LD_PRELOAD. It checks
the NCPU variable if it exists and will present this number of CPUs, or
if it does not exist, will expose the maximum supported number.
2025-01-08 11:26:05 +01:00
Willy Tarreau
bd06502b22 BUILD: makefile: add a qinfo macro to pass info in quiet mode
Some commands such as $(cmd_CC) etc already handle the quiet vs verbose
mode in the makefile, but sometimes we may want to pass other info. The
new "qinfo" macro can be called with a 9-char string argument (spaces
included) as a prefix for some commands, to emit that string when in
quiet mode. The caller must fill the spaces needed for alignment. E.g:

  $(call quinfo,  CC     )$(CC) ...
2025-01-08 11:26:05 +01:00
Willy Tarreau
c87619fa25 BUILD: makefile: do not clean standalone binaries on a simple "make clean"
Running "make clean" currently gets rid of a number of auxiliary tools,
including the standalone ones that do not depend on haproxy's build
options. This is a bit annoying as they have to be rebuilt each time.
Let's move them to the distclean target instead.
2025-01-08 11:26:01 +01:00
William Lallemand
143be1b59f MEDIUM: errors: get rid of shm_open()
Since 5ee266b7 ("MINOR: error: simplify startup_logs_init_shm"), the FD
of the startup logs is always closed and the HAPROXY_STARTUPLOGS_FD
variable is not used anymore. Which means we only need a mmap.

Indeed the shm_open() function was only needed to keep the shm between
the exec() of the master so we can get the logs stored there after doing
the final exec() in wait mode. Since the wait mode doesn't exist
anymore and the parsing is done in a worker, we only need to share a
memory zone between the master and the worker.

This patch removes shm_open() and replace it with a simple mmap(), this
way the shared startup-logs become more portable and USE_SHM_OPEN is not
required anymore.
2025-01-07 16:42:38 +01:00
Frederic Lecaille
d7fc90afe9 BUG/MAJOR: ssl/ocsp: fix NULL conn object dereferencing to access QUIC TLS counters
This bug arrived with this commit in the current dev branch:

	056ec51c26 MEDIUM: ssl/ocsp: counters for OCSP stapling

and could occur for QUIC connections during handshake when the underlying
<conn> connection object is not already initialized. So in this case the TLS
counters attached to TLS listeners cannot be accessed through this object but
from the QUIC connection object.

Modify the code to initialize the listener (<li> variable) for both QUIC
and TCP connections, then initialize the variables for the TLS counters
if the listener is also initialized.

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

Must be backported with the commit mentioned above if it is planned to be
backported.
2025-01-07 15:19:42 +01:00
Christopher Faulet
892eb2bb2c BUG/MEDIUM: promex/resolvers: Don't dump metrics if no nameserver is defined
A 'resolvers' section may be defined without any nameserver. In that case,
we must take care to not dump corresponding Prometheus metrics. However
there is an issue that could lead to a crash or a strange infinite loop
because we are looping on an empty list and, at some point, we are
dereferencing an invalid pointer.

There is an issue because the loop on the nameservers of a resolvers section
is performed via callback functions and not the standard list_for_each_entry
macro. So we must take care to properly detect end of the list and empty
lists for nameservers. But the fix is not so simple because resolvers
sections with and without nameservers may be mixed.

To fix the issue, in rslv_promex_start_ts() and rslv_promex_next_ts(), when the
next resolvers section must be evaluated, a loop is now used to properly skip
empty sections.

This patch is related to #2831. Not sure it fixes it. It must be backported
as far as 3.0.
2025-01-06 09:08:38 +01:00
Amaury Denoyelle
801e39e1cc BUG/MINOR: mux-quic: handle closure of uni-stream
This commit is a direct follow-up to the previous one. As already
described, a previous fix was merged to prevent streamdesc attach
operation on already completed QCS instances scheduled for purging. This
was implemented by skipping app proto decoding.

However, this has a bad side-effect for remote uni-directional stream.
If receiving a FIN stream frame on such a stream, it will considered as
complete because streamdesc are never attached to a uni stream. Due to
the mentionned new fix, this prevent analysis of this last frame for
every uni stream.

To fix this, do not skip anymore app proto decoding for completed QCS.
Update instead qcs_attach_sc() to transform it as a noop function if QCS
is already fully closed before streamdesc instantiation. However,
success return value is still used to prevent an invalid decoding error
report.

The impact of this bug should be minor. Indeed, HTTP3 and QPACK uni
streams are never closed by the client as this is invalid due to the
spec. The only issue was that this prevented QUIC MUX to close the
connection with error H3_ERR_CLOSED_CRITICAL_STREAM.

This must be backported along the previous patch, at least to 3.1, and
eventually to 2.8 if mentionned patches are merged there.
2025-01-03 17:21:19 +01:00
Amaury Denoyelle
af00be8e0f MINOR: mux-quic: change return value of qcs_attach_sc()
A recent fix was introduced to ensure that a streamdesc instance won't
be attached to an already completed QCS which is eligible to purging.
This was performed by skipping application protocol decoding if a QCS is
in such a state. Here is the patch responsible for this change.
  caf60ac696
  BUG/MEDIUM: mux-quic: do not attach on already closed stream

However, this is too restrictive, in particular for unidirection stream
where no streamdesc is never attached. To fix this behavior, first
qcs_attach_sc() API has been modified. Instead of returning a streamdesc
instance, it returns either 0 on success or a negative error code.

There should be no functional changes with this patch. It is only to be
able to extend qcs_attach_sc() with the possibility of skipping
streamdesc instantiation while still keeping a success return value.

This should be backported wherever the above patch has been merged. For
the record, it was scheduled for immediate backport on 3.1, plus merging
on older releases up to 2.8 after a period of observation.
2025-01-03 17:19:21 +01:00
Amaury Denoyelle
4f2554903b BUG/MINOR: mux-quic: fix wakeup on qcc_set_error()
The following patch was a major refactoring of QUIC MUX. It removes
pacing specific code path. In particular, qcc_wakeup() utility function
was removed and replaced by its tasklet_wakup() usage.
  41f0472d96
  MEDIUM: mux-quic: remove pacing specific code on qcc_io_cb

However, an incorrect substitution was performed in qcc_set_error(). As
such, there was no explicit wakeup in case an error is detected by QUIC
MUX or the app protocol layer. This may lead to missing error reporting
to clients.

Fix this by re-add tasklet_wakup() usage into qcc_set_error().

This must be backported up to 3.1 where above patch is scheduled.
2025-01-03 10:39:49 +01:00
Christopher Faulet
f578811c4e MINOR: config: Alert about extra arguments for errorfile and errorloc
errorfile and errorloc directives expect excatly two arguments. But extra
arguments were just ignored while an error should be emitted. It is now
fixed.

This patch could be backported as far as 2.2 if necessary.
2025-01-03 10:10:09 +01:00
Christopher Faulet
a785a20bef BUG/MINOR: log: Allow to use if/unless conditionnals for do-log action
The do-log action does not accept argument for now. But an error was
triggered if any extra arguments was found, preventing the use of if/unless
conditionnals.

When an action is parsed, expected arguments must be tested to detect
missing ones but not unexpected extra arguments because this should be
performed by the conditionnal parser. So just removing the test in the
do-log parser function is enough to fix the issue.

This patch must be backported to 3.1.
2025-01-03 09:44:08 +01:00
Ilia Shipitsin
bbd1cedefc BUG/MINOR: cfgparse-tcp: handle a possible strdup() failure
This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.
2025-01-02 14:31:07 +01:00
Ilia Shipitsin
beca953c55 BUG/MINOR: pool: handle a possible strdup() failure
This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.
2025-01-02 14:31:07 +01:00
Ilia Shipitsin
b4f965be9e BUG/MINOR: compression: handle a possible strdup() failure
This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.
2025-01-02 14:31:07 +01:00
Amaury Denoyelle
caf60ac696 BUG/MEDIUM: mux-quic: do not attach on already closed stream
Due to QUIC packet reordering, a stream may be opened via a new
RESET_STREAM or STOP_SENDING frame. This would cause either Tx or Rx
channel to be immediately closed.

This can cause an issue with current QUIC MUX implementation with QCS
purging. QCS are inserted into QCC purge list when transfer could be
considered as completed. In most cases, this happens after full
request/response exchange. However, it can also happens after request
reception if RESET_STREAM/STOP_SENDING are received first.

A BUG_ON() crash will occur if a STREAM frame is received after. In this
case, streamdesc instance will be attached via qcs_attach_sc() to handle
the new request. However, QCS is already considered eligible to purging.
It could cause it to be released while its streamdesc instance remains.
A BUG_ON() crash detects this problem in qcc_purge_streams().

To fix this, extend qcc_decode_qcs() to skip app proto rcv_buf
invokation if QCS is considered completed. A similar condition was
already implemented when read was previously aborted after a
STOP_SENDING emission by QUIC MUX.

This crash was reproduced on haproxy.org. Here is the output of the
backtrace :
Core was generated by `./haproxy-dev -db -f /etc/haproxy/haproxy-current.cfg -sf 16495'.
Program terminated with signal SIGILL, Illegal instruction.
 #0  0x00000000004e442b in qcc_purge_streams (qcc=0x774cca0) at src/mux_quic.c:2661
2661                    BUG_ON_HOT(!qcs_is_completed(qcs));
[Current thread is 1 (LWP 1457)]
[ ## gdb ## ] bt
 #0  0x00000000004e442b in qcc_purge_streams (qcc=0x774cca0) at src/mux_quic.c:2661
 #1  0x00000000004e4db7 in qcc_io_process (qcc=0x774cca0) at src/mux_quic.c:2744
 #2  0x00000000004e5a54 in qcc_io_cb (t=0x7f71193940c0, ctx=0x774cca0, status=573504) at src/mux_quic.c:2886
 #3  0x0000000000b4f792 in run_tasks_from_lists (budgets=0x7ffdcea1e670) at src/task.c:603
 #4  0x0000000000b5012f in process_runnable_tasks () at src/task.c:883
 #5  0x00000000007de4a3 in run_poll_loop () at src/haproxy.c:2771
 #6  0x00000000007deb9f in run_thread_poll_loop (data=0x1335a00 <ha_thread_info>) at src/haproxy.c:2985
 #7  0x00000000007dfd8d in main (argc=6, argv=0x7ffdcea1e958) at src/haproxy.c:3570

This BUG_ON() crash can only happen since 3.1 refactoring. Indeed, purge
list was only implemented on this version. As such, please backport it
on 3.1 immediately. However, a logic issue remains for older version as
a stream could be attached on a fully closed QCS. Thus, it should be
backported up to 2.8, this time after a period of observation.
2025-01-02 11:25:40 +01:00
Amaury Denoyelle
4a997e5a93 MINOR: mux-quic: add traces on sd attach
Add traces into qcs_attach_sc(). This function is called when a request
is received on a QCS stream and a streamdesc instance is attached. This
will be useful to facilitate debugging.
2025-01-02 11:25:40 +01:00
Amaury Denoyelle
ddfd8031f8 BUG/MAJOR: mux-quic: properly fix BUG_ON on empty STREAM emission
Properly fix BUG_ON() occurence when QUIC MUX emits only empty STREAM
frames. This was addressed by a previous patch but it causes another
regression so a revert was needed.

BUG_ON() on qcc_build_frms() return value is invalid. Indeed,
qcc_build_frms() may return 0, but this does not imply that frame list
is empty, as encoded frames can have a zero length payload. As such,
simply remove this invalid BUG_ON().

This must be backported up to 3.1.
2025-01-02 11:25:40 +01:00
Amaury Denoyelle
85e27f1e92 Revert "BUG/MAJOR: mux-quic: fix BUG_ON on empty STREAM emission"
This reverts commit 9806453742.

Above patch tried to fix a BUG_ON() occurence when MUX only emitted
empty STREAM frames via qcc_build_frms(). Return value of qcs_send() was
changed from the payload STREAM frame to the whole frame length.
However, this is invalid as this return value is used to ensure
connection flow-control is not exceeded on sending retry. This causes
occurence of BUG_ON() crash in qcc_io_send() as send-list is not
properly purged after QCS emission.

Reverts this incorrect fix. The original issue will be properly dealt in
the next commit.

This commit must be backported to 3.1 if reverted commit was already
applied on it.
2025-01-02 11:00:25 +01:00
Christopher Faulet
22f8d2c99e BUG/MEDIUM: mux-h2: Count copied data when looping on RX bufs in h2_rcv_buf()
When data was copied from RX buffers to the channel buffer, more data than
expected could be moved because amount of data copied was never decremented
from the limit. This could lead to a stream dead lock when the compression
filter was inuse.

The issue was introduced by commit 4eb3ff1 ("MAJOR: mux-h2: make streams use
the connection's buffers") but revealed by 3816c38 ("MAJOR: mux-h2: permit a
stream to allocate as many buffers as desired").

Because a h2 stream can now have several RX buffers, in h2_rcv_buf(), we
loop on these buffers to fill the channel buffer. However, we must still
take care to respect the limit to not copy to much data. However, the
"count" variable was never decremented to reflect amount of data already
copied. So, it was possible to exceed the limit.

It was an issue when the compression filter was inuse because the channel
buffer could be fully filled, preventing the compression to be
performed. When this happened, the stream was infinitly blocked because the
compression filter was asking for some space but nothing was scheduled to be
forwarded.

This patch should fix the issue #2826. It must be backported to 3.1.
2025-01-02 09:58:23 +01:00
Amaury Denoyelle
9806453742 BUG/MAJOR: mux-quic: fix BUG_ON on empty STREAM emission
A BUG_ON() is present in qcc_io_send() to ensure that encoded frame list
is empty if qcc_build_frms() previously returned 0.

This BUG_ON() may be triggered if empty STREAM frame is encoded for
standalone FIN. Indeed, qcc_build_frms() returns the sum of all STREAM
payload length. In case only empty STREAM frames are generated, return
value will be 0, despite new frames encoded and inserted into frame
list.

To fix this, change return value of qcs_send(). This now returns the
whole STREAM frame length, both header and payload included. This
ensures that qcc_build_frms() won't return a nul value if new frames are
encoded, even empty ones.

This must be backported up to 3.1.
2024-12-31 16:39:53 +01:00
Aurelien DARRAGON
5bbdd14f56 BUG/MINOR: stktable: invalid use of stkctr_set_entry() with mixed table types
Some actions such as "sc0_get_gpc0" (using smp_fetch_sc_stkctr()
internally) can take an optional table name as parameter to perform the
lookup on a different table from the tracked one but using the key from
the tracked entry. It is done by leveraging the stktable_lookup() function
which was originally meant to perform intra-table lookups.

Calling sc0_get_gpc0() with a different table name will result in
stktable_lookup() being called to perform lookup using a stktsess from
a different table. While it is theorically fine, it comes with a pitfall:
both tables (the one from where the stktsess originates and the actual
target table) should rely on the exact same key type and length.

Failure to do so actually results in undefined behavior, because the key
type and/or length from one table is used to perform the lookup in
another table, while the underlying lookup API expects explicit type and
key length.

For instance, consider the below example:

  peers testpeers
    bind 127.0.0.1:10001
    server localhost

    table test type binary len 1 size 100k expire 1h store gpc0
    table test2 type string size 100k expire 1h store gpc0

  listen test_px
    mode http
    bind 0.0.0.0:8080
    http-request track-sc0 bin(AA) table testpeers/test
    http-request track-sc1 str(ok) table testpeers/test2
    log-format "%[sc0_get_gpc0(testpeers/test2)]"
    log stdout format raw local0

    server s1 git.haproxy.org:80

Performing a curl request to localhost:8080 will cause unitialized reads
because string "ok" from test2 table will be compared as a string against
"AA" binary sample which is not NULL terminated:

==2450742== Conditional jump or move depends on uninitialised value(s)
==2450742==    at 0x484F238: strlen (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2450742==    by 0x27BCE6: stktable_lookup (stick_table.c:539)
==2450742==    by 0x281470: smp_fetch_sc_stkctr (stick_table.c:3580)
==2450742==    by 0x283083: smp_fetch_sc_get_gpc0 (stick_table.c:3788)
==2450742==    by 0x2A805C: sample_process (sample.c:1376)

So let's prevent that by adding some comments in stktable_set_entry()
func description, and by adding a check in smp_fetch_sc_stkctr() to ensure
both source stksess and target table share the same key properties.

While it could be relevant to backport this in all stable versions, it is
probably safer to wait for some time before doing so, to ensure that no
existing configs rely on this ambiguity because the fact that the target
table and source stksess entry need to share the same key type and length
is not explicitly documented.
2024-12-31 16:36:00 +01:00
Aurelien DARRAGON
f94c63021b DOC: config: add missing "track-sc0" in action keywords matrix
In d54e8f8107 ("DOC: config: reorganize actions into their own section"),
"track-sc0" keyword was properly documented but the keyword was not placed
in the action keywords matrix alongside other track-sc* statements. It
was probably overlooked, so let's fix that.

Could be backported up to 2.9 with d54e8f8107.
2024-12-31 16:35:54 +01:00
Willy Tarreau
e148dfd35d [RELEASE] Released version 3.2-dev2
Released version 3.2-dev2 with the following main changes :
    - MINOR: build: define DEBUG_STRESS
    - MINOR: applet: define applet_putchk_stress() alternative
    - MINOR: stats: use stress mode to force reentrant dumps
    - CI: scripts: add support for AWS-LC-FIPS in build-ssl.sh
    - MINOR: ssl: add "FIPS" details in haproxy -vv
    - MEDIUM: ssl: rename 'OpenSSL' by 'SSL library' in haproxy -vv
    - CI: github: let's add an AWS-LC-FIPS job
    - MINOR: window_filter: rely on the time to update the filter samples (QUIC/BBR)
    - BUG/MINOR: quic: wrong logical statement in in_recovery_period() (BBR)
    - BUG/MINOR: quic: fix BBB max bandwidth oscillation issue.
    - BUG/MINOR: quic: wrong bbr_target_inflight() implementation
    - BUG/MINOR: quic: remove max_bw filter from delivery rate sampling
    - BUG/MINOR: quic: underflow issue for bbr_inflight_hi_from_lost_packet()
    - BUG/MINOR: quic: reduce packet losses at least during ProbeBW_CRUISE (BBR)
    - MINOR: quic: reduce the private data size of QUIC cc algos
    - CLEANUP: quic: remove a wrong comment about ->app_limited (drs)
    - BUG/MINOR: quic: fix the wrong tracked recovery start time value
    - BUG/MINOR: quic: too permissive exit condition for high loss detection in Startup (BBR)
    - BUG/MINOR: cli: cli_snd_buf: preserve \r\n for payload lines
    - REGTESTS: ssl: add a PEM with mix of LF and CRLF line endings
    - BUG/MINOR: quic: missing Startup accelerating probing bw states
    - CLEANUP: quic: Rename some BBR functions in relation with bw probing
    - REORG: startup: move global.maxconn calculations in limits.c
    - REORG: startup: move code that applies limits to limits.c
    - REORG: startup: move nofile limit checks in limits.c
    - MINOR: ssl: add utils functions to extract X509 notAfter date
    - MINOR: ssl/cli: allow to filter expired certificates with 'show ssl sni'
    - MINOR: ssl/cli: add -A to the 'show ssl sni' command description
    - BUG/MINOR: ssl/cli: 'show ssl cert' escape the first '*' of a filename
    - BUG/MINOR: ssl/cli: 'show ssl crl-file' escape the first '*' of a filename
    - BUG/MINOR: ssl/cli: 'show ssl ca-file' escape the first '*' of a filename
    - BUG/MEDIUM: stconn: Only consider I/O timers to update stream's expiration date
    - BUG/MEDIUM: queues: Make sure we call process_srv_queue() when leaving
    - BUG/MEDIUM: queues: Do not use pendconn_grab_from_px().
    - CLEANUP: queues: Remove pendconn_grab_from_px().
    - BUILD: debug: only dump/reset glitch counters when really defined
    - MINOR: compiler: add a __has_builtin() macro to detect features more easily
    - MINOR: compiler: rely on builtin detection for __builtin_unreachable()
    - MINOR: compiler: add a new "ASSUME" macro to help the compiler
    - MINOR: compiler: also enable __builtin_assume() for ASSUME()
    - MINOR: compiler: add ASSUME_NONNULL() to tell the compiler a pointer is valid
    - MINOR: bug: make BUG_ON() fall back to ASSUME
    - CLEANUP: cache: use ASSUME_NONNULL() instead of DISGUISE()
    - CLEANUP: hlua: use ASSUME_NONNULL() instead of ALREADY_CHECKED()
    - CLEANUP: htx: use ASSUME_NONNULL() to mark the start line as non-null
    - CLEANUP: mux-fcgi: use ASSUME_NONNULL() to indicate that the first block exists
    - CLEANUP: stats: use ASSUME_NONNULL() to indicate that the first block exists
    - CLEANUP: quic: replace ALREADY_CHECKED() with ASSUME_NONNULL() at a few places
    - CLEANUP: ssl-sock: drop two now unneeded ALREADY_CHECKED()
    - BUG/MEDIUM: mux-quic: do not mix qcc_io_send() return codes with pacing
    - CLEANUP: mux-quic: remove unused qcc member send_retry_list
    - MINOR: quic: add traces
    - MINOR: mux-quic: refactor wait-for-handshake support
    - MEDIUM/OPTIM: mux-quic: define a recv_list for demux resumption
    - MEDIUM/OPTIM: mux-quic: implement purg_list
    - MINOR: mux-quic: extract code to build STREAM frames list
    - MINOR: mux-quic: split STREAM and RS/SS emission
    - MEDIUM/OPTIM: mux-quic: do not rebuild frms list on every send
    - MEDIUM: mux-quic: remove pacing specific code on qcc_io_cb
    - MINOR: trace: implement tracing disabling API
    - MINOR: mux-quic: hide traces when woken up on pacing only
    - MINOR: ssl/cli: add a 'Uncommitted' status for 'show ssl' commands
    - MINOR: ssl/ocsp: Add extra details in error logs when possible
    - BUILD: ssl/ocsp: error: ‘%.*s’ directive argument is null
    - MEDIUM: ssl/ocsp: OCSP response is expired with OCSP_MAX_RESPONSE_TIME_SKEW
    - MINOR: ssl: improve HAVE_SSL_OCSP ifdef
    - DOC: config: add example for server "track" keyword
    - DOC: config: reorder "tune.lua.*" keywords by alphabetical order
    - DOC: config: add "tune.lua.burst-timeout" to the list of global parameters
    - MINOR: hlua: add option to preserve bool type from smp to lua
    - REGTESTS: fix lua-based regtests using tune.lua.smp-preserve-bool
    - BUG/MEDIUM: mux-quic: prevent BUG_ON() by refreshing frms on MAX_DATA
    - CLEANUP: mux-quic: remove dead err label in qcc_build_frms()
    - BUG/MINOR: h2/rhttp: fix HTTP2 conn counters on reverse
    - MINOR: hlua: rename "tune.lua.preserve-smp-bool" to "tune.lua.bool-sample-conversion"
    - MINOR: ssl: change visibility of ssl_stats_module
    - MINOR: ssl: rework the error management in the OCSP callback
    - MEDIUM: ssl/ocsp: counters for OCSP stapling
    - CI: limit aws-lc and libressl Quic Interop to "haproxy" only
    - BUG/MEDIUM: queue: Make process_srv_queue return the number of streams
    - CI: github: try to build the latest WolfSSL master weekly
    - CI: github: activate ASAN on the WolfSSL weekly job
    - BUG/MINOR: stats: fix segfault caused by uninitialized value in "show schema json"
    - MINOR: stktable: add stktable_get_data_type_idx() helper function
    - MINOR: stktable: support optional index for array types in {set, clear, show} table commands
    - CI: scripts: allow to build wolfssl with --enable-debug
    - CI: github: activate debug in wolfssl weekly build
    - BUG/MEDIUM: queues: Stricly respect maxconn for outgoing connections
    - MEDIUM: queue: Handle the race condition between queue and dequeue differently
    - CLEANUP: Remove pendconn_must_try_again().
    - BUILD: compat: add missing fcntl.h before defining F_SETPIPE_SZ
    - BUILD: mworker: always initialize the saveptr of strtok_r()
    - BUILD: limits: make normalize_rlim() take an rlim_t to fix build on m68k
    - BUG/MINOR: checks: handle a possible strdup() failure
    - BUG/MINOR: listener: handle a possible strdup() failure
    - BUG/MINOR: mux_h1: handle a possible strdup() failure
    - BUG/MINOR: debug: handle a possible strdup() failure
2024-12-25 15:17:01 +01:00
Ilia Shipitsin
6524fbfb70 BUG/MINOR: debug: handle a possible strdup() failure
This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.
2024-12-25 12:42:33 +01:00
Ilia Shipitsin
a3e6c783cd BUG/MINOR: mux_h1: handle a possible strdup() failure
This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.
2024-12-25 12:42:33 +01:00
Ilia Shipitsin
89c62693da BUG/MINOR: listener: handle a possible strdup() failure
This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.
2024-12-25 12:41:08 +01:00
Ilia Shipitsin
495f1f9741 BUG/MINOR: checks: handle a possible strdup() failure
This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.
2024-12-25 12:40:56 +01:00
Willy Tarreau
f486f976c7 BUILD: limits: make normalize_rlim() take an rlim_t to fix build on m68k
As can be seen here, the build fails on m68k since commit 665dde648
("MINOR: debug: use LIM2A to show limits") in 3.1:

  https://github.com/haproxy/haproxy/actions/runs/12440234399/job/34735360177

The reason is the comparison between a ulong limit and RLIM_INFINITY.
Indeed, on m68k, rlim_t is an unsigned long long. Let's just change
the function's input type to take an rlim_t instead. This also allows
to get rid of the casts in the call place.

This can be backported to 3.1 though it's not important given the low
prevalence of this platform for such use cases.
2024-12-25 12:33:06 +01:00
Willy Tarreau
21df7677a9 BUILD: mworker: always initialize the saveptr of strtok_r()
Building with some libcs which define strtok_r() as an inline function
can yield a possibly uninitialized warning due to a loop dereferencing
this save pointer early, even though the doc clearly mentions that it
is ignored. This is actually more of a mismatch between the compiler
and the libc (gcc-4.7 and glibc-2.23 in that case). It's trivial to
set s2 to NULL here so let's do it to please this old couple. Note
that while the warning is triggered in all supported versions, there's
no point backporting it since it's unlikely this combination will be
relevant outside of backwards compatibility checks now.
2024-12-25 12:18:46 +01:00
Willy Tarreau
f78121dd32 BUILD: compat: add missing fcntl.h before defining F_SETPIPE_SZ
n 1.5-dev8, 13 years ago, support for setting pipe size was added by
commit bd9a0a778 ("OPTIM/MINOR: make it possible to change pipe size
(tune.pipesize)"). For compatibility purposes, it was defining
F_SETPIPE_SZ in compat.h if it was not set. It apparently always had
F_SETPIPE_SZ defined before being included.

Now in 3.2-dev1, commit fbc534a6f ("REORG: startup: move nofile limit
checks in limits.c") reordered a few includes and ended up with
mworker-prog.c including compat.h before fcntl.h, causing a redefinition
error on certain libcs:

    CC      src/mworker-prog.o
  In file included from /usr/include/bits/fcntl.h:61:0,
                   from /usr/include/fcntl.h:35,
                   from include/haproxy/limits.h:11,
                   from include/haproxy/mworker.h:18,
                   from src/mworker-prog.c:27:
  /usr/include/bits/fcntl-linux.h:203:0: warning: "F_SETPIPE_SZ" redefined [enabled by default]
  In file included from include/haproxy/api-t.h:35:0,
                   from include/haproxy/api.h:33,
                   from src/mworker-prog.c:23:
  include/haproxy/compat.h:161:0: note: this is the location of the previous definition

Let's simply include fcntl.h in compat.h before the macro is redefined.

There's normally no need to backport this, though it's harmless to do
it if needed.
2024-12-25 11:53:11 +01:00