The expression items[JWE_ELT_JOSE].length << 3 performs the shift on an
unsigned int (32-bit) before being cast to uint64_t instead of after.
This means that we don't cover for a possible overflow (which would
never happen as it would need a header length beyond 512MB). At least
fixing it will avoid code check reports.
The allocated ec_R and ec_S were not released in case one of the two
would fail to be allocated/created, and would cause a memory leak. Let's
add the missing BN_free(). This may be backported to 2.4.
The collection of errno in tcpcheck_eval_connect() and tcpcheck_main()
is quite far from the production location, and the risk of having a
zero errno is definitely not null. Tests show that this works, so
better not try to fix something not broken, but at least place a
comment there indicating that it's not necessarily super-reliable.
This would need to be revisited the day we finally store errno in
the connection.
When in 2.2, with commit c8dc20a825 ("BUG/MINOR: checks: refine which
errno values are really errors."), errno reporting was refined, an
extra check was added before calling retrieve_errno_from_socket(), and
by mistake the test on !errno got inverted so that we only call the
function to retrieve the error from the socket when errno is set!
The first test in the function detects it and returns without changing
anything, so this didn't have much effect, however when errno is not
set (certain call places purposely pass zero so that getsockopt() is
used), this wasn't called so the error wasn't reported. Apparently it
only happened when called from process_chk_conn() after an async
error was detected, so probably just cases where POLLERR is reported,
which remains infrequent.
Let's fix the direction of this flag. It can be backported if needed
but it's unlikely anyone really noticed.
When an async connect() fails in sock_conn_check(), it returns an errno
that will not be retrieved later by a subsequent getsockopt(SO_ERROR).
The problem is that this errno is then definitely lost. This is visible
in the 4be_1srv_smtpchk_httpchk_layer47errors regtest that fails on
certain systems (e.g. glibc 2.31 on arm32 running Linux 6.1), where the
connect() error is systematically lost and the "Connection refused" is
never seen in the check status. It also matches a few random reports of
the past indicating that the connection error was sometimes not reported
in the stats page in front of a down server.
Ideally we should store errno in connections as soon as the error is
seen. However this would require significant changes that are not
acceptable yet for 3.4 nor stable releases. A more acceptable fix is to
make use of the extra CO_ER_* flags set by conn_set_errno() as soon as
the error is detected. This will recognize a sufficiently large number
of errors and the check status will report them (here we'll have
"ECONNREFUSED" in the check). Note that on systems where the error is
seen synchronously, we can have "ECONNREFUSED (Connection refused)",
but this is not a problem.
This fix adds the missing conn_set_errno() call to sock_conn_check(),
that is thus sufficient to catch this error. In addition, the two
affected regtests were updated to search for ECONNREFUSED here.
This might be backported to older releases if users request it, but it
is probably not necessary.
While action_inc_gpc1() explicitly checks if s->stkctr or sess->stkctr
are set since 2.8 with commit 6c0117168 ("MEDIUM: stick-table: set the
track-sc limit at boottime via tune.stick-counters"), action_inc_gpc0()
and the generic action_inc_gpc() still stuck to the old approach of not
checking them, causing confusion when reviewing the code.
Upon closer inspection, the only case where the pointer may be NULL is
when global.tune.nb_stk_ctr is zero, which happens when the global
section contains "tune.stick-counters 0". However in this case, the
config parser "parse_inc_gpc()" will reject any reference to any stick
counter, so in theory there is no problem.
Regardless, the difference of treatment between sibling functions remains
confusing and the check is cheap, so let's generalize it, it will save a
future reader from the need to inspect stream_new() and session_new().
Session traces were brought in 3.1 by commit abb07af67 ("MINOR:
session/trace: enable very minimal session tracing") though there was
an issue, because SESS_EV_END and SESS_EV_ERR have the same value (it's
a copy-paste mistake).
This can be backported to 3.2.
When destroying a stream-based DNS nameserver, task_req and task_rsp
were destroyed but task_idle was missed, causing a task object leak.
This doesn't necessarily have to be backported since it's only upon
exit that it is visible.
cfg_parse_resolvers() has many error paths on allocation failure when
parsing "nameserver". These paths handle their own cleanup instead of
centralizing it. The result is that some errors paths leak some fields.
The most complex ones are the strdup() failures which require to check
for stream or dgram to figure what to free. These can be detected via
ASAN on a dummy strdup() allocation failure:
Indirect leak of 131080 byte(s) in 1 object(s) allocated from:
#0 0x7f0b7ed1f0ab in malloc (/usr/lib64/libasan.so.8+0x11f0ab)
#1 0x000000c73e19 in dns_ring_new src/dns_ring.c:59
#2 0x000000af1848 in dns_dgram_init src/dns.c:480
#3 0x000000922005 in cfg_parse_resolvers src/resolvers.c:3792
#4 0x00000089d33e in parse_cfg src/cfgparse.c:2202
#5 0x0000009e0a39 in read_cfg src/haproxy.c:1142
#6 0x000000447e8c in main src/haproxy.c:3474
#7 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
#8 0x7ffd35f1531c ([stack]+0x2031c)
Indirect leak of 304 byte(s) in 1 object(s) allocated from:
#0 0x7f0b7ed1ea23 in calloc (/usr/lib64/libasan.so.8+0x11ea23)
#1 0x000000af1681 in dns_dgram_init src/dns.c:468
#2 0x000000922005 in cfg_parse_resolvers src/resolvers.c:3792
#3 0x00000089d33e in parse_cfg src/cfgparse.c:2202
#4 0x0000009e0a39 in read_cfg src/haproxy.c:1142
#5 0x000000447e8c in main src/haproxy.c:3474
#6 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
#7 0x7ffd35f1531c ([stack]+0x2031c)
Indirect leak of 104 byte(s) in 1 object(s) allocated from:
#0 0x7f0b7ed1ea23 in calloc (/usr/lib64/libasan.so.8+0x11ea23)
#1 0x000000921f83 in cfg_parse_resolvers src/resolvers.c:3772
#2 0x00000089d33e in parse_cfg src/cfgparse.c:2202
#3 0x0000009e0a39 in read_cfg src/haproxy.c:1142
#4 0x000000447e8c in main src/haproxy.c:3474
#5 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
#6 0x7ffd35f1531c ([stack]+0x2031c)
Indirect leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x7f0b7ed1f0ab in malloc (/usr/lib64/libasan.so.8+0x11f0ab)
#1 0x000000c73e09 in dns_ring_new src/dns_ring.c:55
#2 0x000000af1848 in dns_dgram_init src/dns.c:480
#3 0x000000922005 in cfg_parse_resolvers src/resolvers.c:3792
#4 0x00000089d33e in parse_cfg src/cfgparse.c:2202
#5 0x0000009e0a39 in read_cfg src/haproxy.c:1142
#6 0x000000447e8c in main src/haproxy.c:3474
#7 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
#8 0x7ffd35f1531c ([stack]+0x2031c)
Indirect leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x7f0b7ed18e20 in strdup (/usr/lib64/libasan.so.8+0x118e20)
#1 0x00000092203b in cfg_parse_resolvers src/resolvers.c:3798
#2 0x00000089d33e in parse_cfg src/cfgparse.c:2202
#3 0x0000009e0a39 in read_cfg src/haproxy.c:1142
#4 0x000000447e8c in main src/haproxy.c:3474
#5 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
#6 0x7ffd35f1531c ([stack]+0x2031c)
This should be completely reworked so that the cleanup is performed in
a central place, as the risk to get it wrong remains high.
This patch does the minimal changes to clean this up. It does not need
to be backported since it only triggers on boot OOM.
Some strdup() failures in parse_resolve_conf() do not release everything
due to the way the function is built, resulting in leaks on error that are
caught by ASAN:
Direct leak of 304 byte(s) in 1 object(s) allocated from:
#0 0x7fe74231ea23 in calloc (/usr/lib64/libasan.so.8+0x11ea23)
#1 0x000000af1681 in dns_dgram_init src/dns.c:468
#2 0x00000091cbbf in parse_resolve_conf src/resolvers.c:3559
#3 0x00000092179e in cfg_parse_resolvers src/resolvers.c:3815
#4 0x00000089d33e in parse_cfg src/cfgparse.c:2202
#5 0x0000009e0a39 in read_cfg src/haproxy.c:1142
#6 0x000000447e8c in main src/haproxy.c:3474
#7 0x7fe74162ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
#8 0x7ffc0a43e31f ([stack]+0x2031f)
Indirect leak of 131080 byte(s) in 1 object(s) allocated from:
#0 0x7fe74231f0ab in malloc (/usr/lib64/libasan.so.8+0x11f0ab)
#1 0x000000c73e19 in dns_ring_new src/dns_ring.c:59
#2 0x000000af1848 in dns_dgram_init src/dns.c:480
#3 0x00000091cbbf in parse_resolve_conf src/resolvers.c:3559
#4 0x00000092179e in cfg_parse_resolvers src/resolvers.c:3815
#5 0x00000089d33e in parse_cfg src/cfgparse.c:2202
#6 0x0000009e0a39 in read_cfg src/haproxy.c:1142
#7 0x000000447e8c in main src/haproxy.c:3474
#8 0x7fe74162ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
#9 0x7ffc0a43e31f ([stack]+0x2031f)
Indirect leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x7fe74231f0ab in malloc (/usr/lib64/libasan.so.8+0x11f0ab)
#1 0x000000c73e09 in dns_ring_new src/dns_ring.c:55
#2 0x000000af1848 in dns_dgram_init src/dns.c:480
#3 0x00000091cbbf in parse_resolve_conf src/resolvers.c:3559
#4 0x00000092179e in cfg_parse_resolvers src/resolvers.c:3815
#5 0x00000089d33e in parse_cfg src/cfgparse.c:2202
#6 0x0000009e0a39 in read_cfg src/haproxy.c:1142
#7 0x000000447e8c in main src/haproxy.c:3474
#8 0x7fe74162ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
#9 0x7ffc0a43e31f ([stack]+0x2031f)
SUMMARY: AddressSanitizer: 131448 byte(s) leaked in 3 allocation(s).
Let's free the dgram and the dns ring. This can be backported though it's
not important as it only happens on OOM condition during boot.
When an expression is used for do-resolve(), an error may be reported.
Unfortunately it was scratched and replaced by the do-resolve() error,
leaving no chance to know exactly what was wrong. Let's report the
contents of the error when available. It will indicate identifiers that
are not found or invalid ranges or types being used.
This can be backported to all versions.
The sample used to pass the IP address only had its data, px, sess and
strm fields initialized before being passed to vars_set_by_name(). It
turns out that this latter one doesn't seem to touch ctx, flags nor opt
but nothing guarantees it. Let's at least initialize the fields properly
to avoid passing random garbage.
No backport is needed.
In parse_new_proxy(), when proxy_defproxy_cpy() fails, the error path used
ha_free(&curproxy) to release the partially constructed proxy. However, the
proxy was allocated via alloc_new_proxy() which performs significant setup:
- setup_new_proxy() inserts it into the proxy_by_name tree (proxy_store_name)
- It appends to the global proxies list (LIST_APPEND)
- proxy_take() increments its refcount
Additionally, proxy_defproxy_cpy() may have allocated further resources
(strdup'd strings, compression structures, email alert fields, etc).
Using ha_free() only freed the proxy struct itself, leaving:
- The proxy still registered in the name tree (dangling pointer)
- The proxy still linked in the global proxies list
- All strdup'd strings and other allocations leaked
This is visible with ASAN when causing random allocation errors:
[NOTICE] (27033) : haproxy version is 3.4-dev12-b15468-11
[NOTICE] (27033) : path to executable is ./haproxy
[ALERT] (27033) : config : parsing [/dev/stdin:5015] : proxy 'bk3': failed to duplicate tcpcheck preset-vars
[ALERT] (27033) : config : Error(s) found in configuration file : /dev/stdin
=================================================================
==27033==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 4 byte(s) in 1 object(s) allocated from:
#0 0x7f113e518e20 in strdup (/usr/lib64/libasan.so.8+0x118e20)
#1 0x000000955410 in setup_new_proxy src/proxy.c:3178
#2 0x000000955816 in alloc_new_proxy src/proxy.c:3221
#3 0x000000956c33 in parse_new_proxy src/proxy.c:3554
#4 0x000000a24d03 in cfg_parse_listen src/cfgparse-listen.c:495
#5 0x00000089d33e in parse_cfg src/cfgparse.c:2202
#6 0x0000009e0bb9 in read_cfg src/haproxy.c:1142
#7 0x000000447e8c in main src/haproxy.c:3474
#8 0x7f113d82ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
#9 0x7fff65b4e320 ([stack]+0x20320)
SUMMARY: AddressSanitizer: 4 byte(s) leaked in 1 allocation(s).
The fix replaces ha_free(&curproxy) with proxy_drop(curproxy), which
properly calls deinit_proxy() to release all internal resources, removes
the proxy from trees and lists, decrements the refcount, and frees the
struct.
No backport is needed since proxy_drop() is only in 3.4.
In dns_dgram_init(), the newly created dgram is assigned to the name server
before the ring is attached. In case of errors, e.g. due to too many watchers,
the dgram is released but not removed from ns->dgram. Let's only assign the
pointer on success to avoid this, as it's not needed before. The problem
was introduced in 2.4 with commit c943799c86 ("MEDIUM: resolvers/dns: split
dns.c into dns.c and resolvers.c"), and was possibly there before. The fix
may be backported to all stable versions.
The resolver 'r' is appended to the global sec_resolvers list, but upon failure
later, pointers are released but the element remains in the list, corrupting it,
and possibly causing a crash during deinit() when releasing remaining ones.
Adding a LIST_DEL_INIT() on the error unrolling path is sufficient.
Note that the issue will only happen on failure to allocate memory via
strdup() so the risk is low. The bug was introduced in 2.6 by commit
e7f5776800 ("MINOR: resolvers: resolvers_new() create a resolvers with
default values"), so the fix may be backported to several releases, but
does not necessarily have to go that far.
In cli_parse_set_server()'s 'ssl' branch, the server lock is taken,
and not released in case srv_set_ssl() fails, resulting in a dead lock
and a panic the next time an attempt to touch this server is made. The
lock must be released on all error paths.
This was introduced in 3.3 by commit f8f94ffc9 ("BUG/MEDIUM: server:
Use sni as pool connection name for SSL server only") which was marked
for backporting to 3.0, so this must likely be backported that far.
The condition 'if (srv->pool_conn_name)' was checking the destination
instead of the source 'src->pool_conn_name', meaning the strdup() would
never fire (since newly calloc'd servers start with NULL pool_conn_name),
and the pool_conn_name setting from default-server was silently ignored.
Introduced in 3.2 with commit f0f1816f1 ("MINOR: check: implement
check-pool-conn-name srv keyword") when pool_conn_name support was added
to srv_settings_cpy(). The bug caused any 'pool-conn-name' setting in a
'default-server' line to be lost for all servers inheriting from it.
Note that it's not the first time this function induces such a bug due
to the poor choice of "srv" vs "src" that should be renamed to avoid
keyboard mistakes and visual confusion.
This needs to be backported to 3.2.
This function may face an OOM on strdup() in the middle of the hostname
or hostname_dn replacement, leaving NULLs in either or both of the server's
fields, which is definitely not good for other call places.
Let's perform a safe replacement instead: we first allocate the new
values, and only if they are successful, then we release the previous
ones and replace them.
It is not necessary to backport this unless the issue is reported (it
was found via code review).
All task allocation errors report "memory allocation error initializing
the ring" when the actual failure was task_new_anywhere() returning NULL.
This clearly is a copy-paste. Let's fix the error messages to help when
debugging. Since it's only about allocation failures during init, there
is probably no point in backporting this.
One is s/keyworld/keyword in the retry-on parser. The other one is a
wrong argument "len" being printed in case of parse error for
"declare capture" instead of the length itself.
These can be backported though they are not important.
In dns_session_init(), sockaddr_alloc() allocates 'addr' from the sockaddr
pool, but on failure of appctx_finalize_startup() we jump to the error label
without calling sockaddr_free(&addr), leaking the allocation. Let's add the
missing sockaddr_free() on the error branch.
This must be backported to 2.6.
The original DNS code would only use the 8 lower bits of the compression
offset. This was fixed in 2.0 with commit 2fa66c3b9 ("BUG/MEDIUM: dns:
overflowed dns name start position causing invalid dns error") but it was
not sufficient because the anti-loop check continues to use only 8 of the
14 bits, thus a crafted response where the 8 lower bits pass the check and
the 6 higher should fail it would be accepted. The impacts remains limited
thanks to the bounds check and the recursion limits, but such invalid
responses could still cost a lot to process. Let's compute the 14-bit
offset once for all and use it everywhere.
In resolv_validate_dns_response(), the additional records loop calls
resolv_read_name(). When it returns zero due to a bad response, the main
loop does a "continue" without making the "reader" pointer progress, so it
evaluates the exact same field again and again. Fortunately this is limited
by arcount which is 16 bits, but it means it can still iterate 65535 times
there, allocating and releasing an answer_record at each turn. Let's just
jump to the invalid_resp label that handles the cleaning. There was the
same pattern (without the allocation) with nscount a few lines above BTW.
These can possibly explain some situations where a high CPU usage observed
processing responses.
Seems like these were introduced in 2.2 with commit 37950c8d2
("BUG/MEDIUM: dns: improper parsing of aditional records")
This must be backported to stable versions.
In cfg_parse_resolvers(), two duplicate name checks set err_code but lacked
'goto out', allowing execution to fall through and create the duplicate entry.
This would result in new resolvers and nameservers to be created after the
error was displayed, and a leak of the previous one. It's mostly harmless
since we're exiting after such errors. This can be backported if desired.
In the inner while loop that validates each character of a POST parameter
value, the code checks *p via HTTP_IS_TOKEN() and HTTP_IS_LWS() instead
of *end, while the loop condition only advances "end", so only the first
character of each value is validated.
This means spaces or binary data embedded in parameter values after the
first character goes undetected. Fix by replacing both references to *p
with *end to properly scan through all characters as intended.
This bug was introduced in 1.5-dev20 by commit 98634f0c7 ("MEDIUM:
backend: Enhance hash-type directive with an algorithm options") so
the fix must be backported to all versions.
In domain-based routing and policy rules, suffix matching on hostnames is
often easier to express as a prefix match on reversed labels. A dedicated
converter makes this convenient with existing fetches and matchers.
This also has a performance benefit for large maps. Prefix string matches use
the prefix-tree index (PAT_MATCH_BEG with pat_idx_tree_pfx), while end matches
use the string-list index (PAT_MATCH_END with pat_idx_list_str), so
reversed-label lookups can avoid linear suffix scans.
This patch adds "reverse_dom", a string converter that reverses domain labels,
ignores one optional trailing dot on input, and rejects empty labels. It
intentionally leaves trailing-dot handling to the caller so configurations can
choose between exact matches, subdomain-only matches, or an explicit dotted
form built with "concat(.)" for prefix lookups.
Examples:
example.com -> com.example
mail.example.com -> com.example.mail
The documentation is updated and a reg-test covers the converter itself, the
explicit dotted form for "map_beg()", and the subdomain-only "-m beg" case.
Some use cases benefit from reversing a string before passing it to other
converters or lookups. While reverse_dom addresses domain-specific label
reversal, a generic byte-wise string reversal remains useful on its own and can
also be combined with other converters such as concat().
A common lookup use case is turning a suffix match on the original string into
a prefix match on the reversed string. Prefix string matches use the
prefix-tree index (PAT_MATCH_BEG with pat_idx_tree_pfx), while end matches use
the string-list index (PAT_MATCH_END with pat_idx_list_str), so reversing
before map_beg can avoid linear suffix scans for large maps.
This patch adds a new string converter named "reverse". It reverses the input
string byte by byte and returns the resulting string unchanged otherwise. It
does not apply any domain-specific semantics or character-encoding semantics.
The documentation is updated and a reg-test is added to cover the basic
conversion as well as a simple composition with concat(.).
This patch is part of a renaming affecting mux_quic and related files.
Naming "qcm" will become the new marker for common mux_quic stuffs,
shared both on QUIC and QMux protocols. The final objective is to limit
"qmux" naming for stuffs related to the new experimental protocol of the
same name.
The current patch renames mux_quic traces token to "qcm". This is the
only change with external visible impacts in the whole renaming series.
However, to preserve compatibility, trace source alias is defined with
the older name "qmux". This relies on the previous patch which
introduced "alias" new trace_source member.
Add a new "alias" member in trace_source structure. Its purpose is to be
an alternative to the member "name". This will be used in the next patch
to allow renaming of QUIC mux traces while preserving compatibility.
This new member is only used in trace_find_source() which is the helper
used to retrieve a trace source from its name.
This is a follow-up on the QUIC MUX renaming process.
The current patch performs renaming in xprt_qmux layer. Older "qstrm"
identifier is replaced by the new name "qmux". Every remaining functions
and structures in xprt_qmux are changed. Outside effects are only
present in QUIC MUX which directly uses some of these functions.
This is a follow-up on the QUIC MUX renaming process.
The current patch performs renaming of "qstrm" to "qmux" in connection
flags. These flags are only used in linked with the xprt_qmux layer.
This has an impact on every files which manipulates these flags, namely
backend, session and ssl_sock sources.
Also, internal xprt identifier is renamed from XPRT_QSTRM to XPRT_QMUX,
This is a follow-up on the QUIC MUX renaming process.
The current patch replaces "qstrm" naming with "qmux" in QUIC MUX source
file. Some members are impacted in qcc and qcs structures, as well as
some internal functions used for QMux receive/send. Internal mux_ops is
also rename to qmux_ops. This is not a breaking change as its externally
visible name was already set to "qmux" originally.
This is a follow-up on the QUIC MUX renaming process. Now most of "qmux"
generic usages as been replaced in favor of "qcm" naming. The next part
of the renaming is to replace "qstrm" naming with "qmux" for stuffs
related to the new QMux protocol specifically.
This is first applied on filenames. As with the previous renaming,
Makefile and include statements are updated as well to prevent
compilation issues.
This is a follow-up on the QUIC MUX renaming process.
This patch renames several definitions in QUIC MUX traces source code.
This is only an internal change. Trace source name is kept to "qmux" for
now, but will be changed in a dedicated patch.
This is a follow-up on the QUIC MUX renaming process.
A previous patch already renames MUX ops callbacks. The current patch
proceed to a similar operation for the rest of the MUX functions.
This is a follow-up on the QUIC MUX renaming process.
The current patch renames all MUX functions used as stream ops
callbacks. Also, internally defined mux_ops is also renamed from
"qmux_ops" to "quic_ops". There is no breaking change as mux_ops name
field remain set to "QUIC".
This patch is the first one of the renaming serie, affecting the QUIC
MUX module. The objective is to remove older "qmux" naming which was
used as a generic identifier. Now it should be restricted to the QMux
experimental protocol. A new "qcm" naming will replace the generic
usage.
The current patch renames the files themselves. Token "qmux" is replaced
by the new "qcm" identifier. Makefile and include statements are
adjusted as required.
This patch adds the missing include of xprt_qstrm header into its
companion source file. This helped to detect an incoherence in the
xprt_qstrm_xfer_rxbuf() prototype which is now fixed.
Header files is also updated with mandatory include statements and
forward declaration.
No backport needed.
Since http-client applet uses its own buffers, it is possible to have data
stuck in the applet input buffer while the http-client response buffer is
full, preventing the applet to consume these data. If this happens on the
last part of the response payload, the upper stream can decide to shut the
applet. In this case, the applet using the http client will not be able to
retrieve these last data because they will never be move into the hc
response buffer.
The main reason for this bug is that, for now, the applets cannot survive
the upper stream unlike multiplexers. It could be a good improvement for the
3.5. However, some applets still uses the stream-connector and the upper
stream (peer and stat applets for instance). So it is not an easy task.
In the mean time, to fix the issue on stable branches, the http-client
applet now stops to consume data when the hc response buffer is not empty.
This way, the applet shut will be deferred. Data will be consumed when they
can be fully moved in the httpclient response buffer.
This patch should fix the issue #3366. It must be backported to 3.3.
In dict_insert(), the read lock on d->rwlock was released before
incrementing the entry's refcount. Between the RDUNLOCK and the
HA_ATOMIC_INC, another thread could call dict_entry_unref() to drop
the refcount to zero, acquire the write lock, delete the entry from
the tree, and free it. The subsequent HA_ATOMIC_INC would then be a
use-after-free on freed memory.
The fix moves the HA_ATOMIC_INC inside the read lock, matching the
pattern used in stick_table.c for identical refcount-then-unlock
sequences.
It can be backported to the branches where this is relevant.
Some outdated comments, as well as typos were fixed in the following files:
dgram.h protocol.h queue-t.h cpu_topo.c debug.c dict.c
protocol.c queue.c raw_sock.c trace.c wdt.c
The "<tgid>/" and "/" wildcard forms previously produced no output.
This isn't a bug since they are new, but a script written for future
versions (where the slash form will gain per-thread-group semantics)
would not work the same on 3.4. Make them produce output by dropping
the redundant ctx->fd = -1 wildcard sentinel; also tighten tgid
validation to reject values <= 0.
Cppcheck found the issue described in github #2124, which can cause these
errors if no CPUSET implementation is supported (and CPUSET_USE_ULONG is
not enabled):
src/cpuset.c:21:11: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c:36:11: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c💯1: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c:124:1: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c:152:1: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c:163:1: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
This can be backported.
In regex_comp(), the error variable is either a const char* (USE_PCRE)
or a a uchar[] (USE_PCRE2), and navigating through the ifdefs is quite a
mess, making it hard to figure if it's always properly initialized when
printing an error message. Let's just preset it to NULL to clarify what
comes from where.
Latest commit 2dfbc311a8 ("BUG/MINOR: uri-auth: avoid leaks on
initialization error") left a possible null-deref case which was
surprisingly only detected by certain compiler combinations. No
backport needed.
