Re: [PATCH 3/3] BUG/MINOR: haproxy: Free rule->arg.vars.expr during deinit_act_rules
Hi Tim, On Sun, Jun 14, 2020 at 12:37:43AM +0200, Tim Duesterhus wrote: > Willy, > > careful with this one: I don't know whether it's safe to simply free the > expression there or whether I need to somehow check whether there actually > is some expression. > > It does not crash with my stupid example configuration showcasing the leak, > but of course real world configurations might or might not trigger a bogus > free there. It seems to be OK. I was worried that the pointer could be part of a union containing either the expression or its text version during parsing, but this doesn't seem to be the case, so apparently it's OK to always free it if it appears in a rule. Thanks! Willy
[PATCH 3/3] BUG/MINOR: haproxy: Free rule->arg.vars.expr during deinit_act_rules
Willy,
careful with this one: I don't know whether it's safe to simply free the
expression there or whether I need to somehow check whether there actually
is some expression.
It does not crash with my stupid example configuration showcasing the leak,
but of course real world configurations might or might not trigger a bogus
free there.
Best regards
Tim Düsterhus
Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --
Given the following example configuration:
frontend foo
bind *:8080
mode http
http-request set-var(txn.foo) str(bar)
Running a configuration check within valgrind reports:
==23665== Memcheck, a memory error detector
==23665== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==23665== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==23665== Command: ./haproxy -c -f ./crasher.cfg
==23665==
[WARNING] 165/002941 (23665) : config : missing timeouts for frontend 'foo'.
| While not properly invalid, you will certainly encounter various
problems
| with such a configuration. To fix this, please ensure that all
following
| timeouts are set to a non-zero value: 'client', 'connect', 'server'.
Warnings were found.
Configuration file is valid
==23665==
==23665== HEAP SUMMARY:
==23665== in use at exit: 314,008 bytes in 87 blocks
==23665== total heap usage: 160 allocs, 73 frees, 1,448,074 bytes
allocated
==23665==
==23665== 132 (48 direct, 84 indirect) bytes in 1 blocks are definitely
lost in loss record 15 of 28
==23665==at 0x4C2FB55: calloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==23665==by 0x4A2612: sample_parse_expr (sample.c:876)
==23665==by 0x54DF84: parse_store (vars.c:766)
==23665==by 0x528BDF: parse_http_req_cond (http_rules.c:95)
==23665==by 0x469F36: cfg_parse_listen (cfgparse-listen.c:1339)
==23665==by 0x459E33: readcfgfile (cfgparse.c:2167)
==23665==by 0x5074FD: init (haproxy.c:2021)
==23665==by 0x418262: main (haproxy.c:3126)
==23665==
==23665== LEAK SUMMARY:
==23665==definitely lost: 48 bytes in 1 blocks
==23665==indirectly lost: 84 bytes in 2 blocks
==23665== possibly lost: 0 bytes in 0 blocks
==23665==still reachable: 313,876 bytes in 84 blocks
==23665== suppressed: 0 bytes in 0 blocks
==23665== Reachable blocks (those to which a pointer was found) are not
shown.
==23665== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==23665==
==23665== For counts of detected and suppressed errors, rerun with: -v
==23665== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
After this patch is applied the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
---
src/haproxy.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/haproxy.c b/src/haproxy.c
index 6548db6b5..245ac3b60 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2555,6 +2555,7 @@ static void deinit_act_rules(struct list *rules)
list_for_each_entry_safe(rule, ruleb, rules, list) {
LIST_DEL(>list);
deinit_acl_cond(rule->cond);
+ release_sample_expr(rule->arg.vars.expr);
if (rule->release_ptr)
rule->release_ptr(rule);
free(rule);
--
2.27.0
[PATCH 1/3] MINOR: haproxy: Add void deinit_and_exit(int)
This helper function calls deinit() and then exit() with the given status.
---
include/haproxy/global.h | 1 +
src/haproxy.c| 5 +
2 files changed, 6 insertions(+)
diff --git a/include/haproxy/global.h b/include/haproxy/global.h
index 4cf537403..1028c1b2a 100644
--- a/include/haproxy/global.h
+++ b/include/haproxy/global.h
@@ -61,6 +61,7 @@ struct proxy;
struct server;
int main(int argc, char **argv);
void deinit(void);
+void deinit_and_exit(int);
void run_poll_loop(void);
int tell_old_pids(int sig);
int delete_oldpid(int pid);
diff --git a/src/haproxy.c b/src/haproxy.c
index dcde0c100..2250815b3 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2865,6 +2865,11 @@ void deinit(void)
deinit_pollers();
} /* end deinit() */
+void deinit_and_exit(int status)
+{
+ deinit();
+ exit(status);
+}
/* Runs the polling loop */
void run_poll_loop()
--
2.27.0
[PATCH 2/3] MINOR: haproxy: Make use of deinit_and_exit() for clean exits
Particularly cleanly deinit() after a configuration check to clean up the output of valgrind which reports "possible losses" without a deinit() and does not with a deinit(), converting actual losses into proper hard losses which makes the whole stuff easier to analyze. As an example, given an example configuration of the following: frontend foo bind *:8080 mode http Running `haproxy -c -f cfg` within valgrind will report 4 possible losses: $ valgrind --leak-check=full ./haproxy -c -f ./example.cfg ==21219== Memcheck, a memory error detector ==21219== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. ==21219== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info ==21219== Command: ./haproxy -c -f ./example.cfg ==21219== [WARNING] 165/001100 (21219) : config : missing timeouts for frontend 'foo'. | While not properly invalid, you will certainly encounter various problems | with such a configuration. To fix this, please ensure that all following | timeouts are set to a non-zero value: 'client', 'connect', 'server'. Warnings were found. Configuration file is valid ==21219== ==21219== HEAP SUMMARY: ==21219== in use at exit: 1,436,631 bytes in 130 blocks ==21219== total heap usage: 153 allocs, 23 frees, 1,447,758 bytes allocated ==21219== ==21219== 7 bytes in 1 blocks are possibly lost in loss record 5 of 54 ==21219==at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==21219==by 0x5726489: strdup (strdup.c:42) ==21219==by 0x468FD9: bind_conf_alloc (listener.h:158) ==21219==by 0x468FD9: cfg_parse_listen (cfgparse-listen.c:557) ==21219==by 0x459DF3: readcfgfile (cfgparse.c:2167) ==21219==by 0x5056CD: init (haproxy.c:2021) ==21219==by 0x418232: main (haproxy.c:3121) ==21219== ==21219== 14 bytes in 1 blocks are possibly lost in loss record 9 of 54 ==21219==at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==21219==by 0x5726489: strdup (strdup.c:42) ==21219==by 0x468F9B: bind_conf_alloc (listener.h:154) ==21219==by 0x468F9B: cfg_parse_listen (cfgparse-listen.c:557) ==21219==by 0x459DF3: readcfgfile (cfgparse.c:2167) ==21219==by 0x5056CD: init (haproxy.c:2021) ==21219==by 0x418232: main (haproxy.c:3121) ==21219== ==21219== 128 bytes in 1 blocks are possibly lost in loss record 35 of 54 ==21219==at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==21219==by 0x468F90: bind_conf_alloc (listener.h:152) ==21219==by 0x468F90: cfg_parse_listen (cfgparse-listen.c:557) ==21219==by 0x459DF3: readcfgfile (cfgparse.c:2167) ==21219==by 0x5056CD: init (haproxy.c:2021) ==21219==by 0x418232: main (haproxy.c:3121) ==21219== ==21219== 608 bytes in 1 blocks are possibly lost in loss record 46 of 54 ==21219==at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==21219==by 0x4B953A: create_listeners (listener.c:576) ==21219==by 0x4578F6: str2listener (cfgparse.c:192) ==21219==by 0x469039: cfg_parse_listen (cfgparse-listen.c:568) ==21219==by 0x459DF3: readcfgfile (cfgparse.c:2167) ==21219==by 0x5056CD: init (haproxy.c:2021) ==21219==by 0x418232: main (haproxy.c:3121) ==21219== ==21219== LEAK SUMMARY: ==21219==definitely lost: 0 bytes in 0 blocks ==21219==indirectly lost: 0 bytes in 0 blocks ==21219== possibly lost: 757 bytes in 4 blocks ==21219==still reachable: 1,435,874 bytes in 126 blocks ==21219== suppressed: 0 bytes in 0 blocks ==21219== Reachable blocks (those to which a pointer was found) are not shown. ==21219== To see them, rerun with: --leak-check=full --show-leak-kinds=all ==21219== ==21219== For counts of detected and suppressed errors, rerun with: -v ==21219== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0) Re-running the same command with the patch applied will not report any losses any more: $ valgrind --leak-check=full ./haproxy -c -f ./example.cfg ==22124== Memcheck, a memory error detector ==22124== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. ==22124== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info ==22124== Command: ./haproxy -c -f ./example.cfg ==22124== [WARNING] 165/001503 (22124) : config : missing timeouts for frontend 'foo'. | While not properly invalid, you will certainly encounter various problems | with such a configuration. To fix this, please ensure that all following | timeouts are set to a non-zero value: 'client', 'connect', 'server'. Warnings were found. Configuration file is valid ==22124== ==22124== HEAP SUMMARY:
Re: [PATCH] switch to clang-9 in Linux/travis-ci builds
On Sun, Jun 14, 2020 at 01:20:43AM +0500, ??? wrote: > I added "-O1" to travis builds. > Can we apply it until a better solution will be found ? Applied, thank you Ilya. Willy
Re: [PATCH] switch to clang-9 in Linux/travis-ci builds
I added "-O1" to travis builds.
Can we apply it until a better solution will be found ?
пт, 12 июн. 2020 г. в 21:40, Илья Шипицин :
>
>
> пт, 12 июн. 2020 г. в 21:09, Willy Tarreau :
>
>> On Fri, Jun 12, 2020 at 08:57:44PM +0500, ??? wrote:
>> > ??, 12 ???. 2020 ?. ? 20:46, Willy Tarreau :
>> >
>> > > On Fri, Jun 12, 2020 at 08:11:52PM +0500, ??? wrote:
>> > > > > Has it ever reported a *real* issue ? I mean, we've been working
>> around
>> > > > >
>> > > >
>> > > >
>> > > > https://github.com/haproxy/haproxy/issues/96
>> > > > https://github.com/haproxy/haproxy/issues/104
>> > > > https://github.com/haproxy/haproxy/issues/106
>> > > > https://github.com/haproxy/haproxy/issues/111
>> > >
>> > > Well only two above are the address sanitizer, one is valgrind and the
>> > > other one is the thread sanitizer.
>> > >
>> > > > and I hope that William Lallemand also found and fixed about 5 bugs
>> > > > detected by travis + asan
>> > >
>> > > Maybe.
>> > >
>> > > In that case at least we should run it at -O1. It's at -O2 that it
>> > > produces bogus code from what I'm seeing. And given that the docs
>> > > also suggest -O1 to get a usable backtrace, it's possible that it's
>> > > rarely tested in combination with -O2. But anyway I really *hate*
>> > > seeing compilers silently emit bad code, especially when it happens
>> > > when asking them to detect more anomalies!
>> > >
>> >
>> > I may try to report it. Is there small repro code ?
>>
>> Sadly not, as usual with bad code. It dies in b_alloc_margin() with a
>> NULL "buf" (long after having successfully dereferenced it) when getting
>> the first H2 request. Putting a printf() in the caller (h2_get_buf) is
>> usually enough to stop the bug. However a printf in the caller doesn't
>> change anything, which could indicate that we may succeed in isolating
>> these functions. I saw it crash when buf was assigned to rbx register,
>> maybe it's occasionally clobbered by their functions, I don't know.
>> I've spent way too much time on it now. I may try to elaborate a repro
>> later but I have way more important things to do than trying to debug
>> an experimental tool that's broken in other areas anyway :-/
>>
>
>
> I'll play with recent builds on travis switching -O2 <--> -O1
>
>
>>
>> Willy
>>
>
From fc74deb80c15144a6649729a4ec92498f606206f Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin
Date: Sun, 14 Jun 2020 01:08:37 +0500
Subject: [PATCH] CI: travis-ci: "-O1" for clang builds
it turned out that clang asan fails with -O2 option
better solution yet to be found
ML: https://www.mail-archive.com/haproxy@formilux.org/msg37621.html
---
.travis.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.travis.yml b/.travis.yml
index 809c2292f..6199bc881 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -119,7 +119,7 @@ install:
script:
- if [ "${CC%-*}" = "clang" ]; then export FLAGS="$FLAGS USE_OBSOLETE_LINKER=1" DEBUG_CFLAGS="-g -fsanitize=address" LDFLAGS="-fsanitize=address"; fi
- make -C contrib/wurfl
- - make -j3 CC=$CC V=1 ERR=1 TARGET=$TARGET $FLAGS DEBUG_CFLAGS="$DEBUG_CFLAGS" LDFLAGS="$LDFLAGS" ADDLIB="-Wl,-rpath,$SSL_LIB" 51DEGREES_SRC="$FIFTYONEDEGREES_SRC" EXTRA_OBJS="$EXTRA_OBJS" $DEBUG_OPTIONS
+ - make -j3 CC=$CC CPU_CFLAGS.generic="-O1" V=1 ERR=1 TARGET=$TARGET $FLAGS DEBUG_CFLAGS="$DEBUG_CFLAGS" LDFLAGS="$LDFLAGS" ADDLIB="-Wl,-rpath,$SSL_LIB" 51DEGREES_SRC="$FIFTYONEDEGREES_SRC" EXTRA_OBJS="$EXTRA_OBJS" $DEBUG_OPTIONS
- ./haproxy -vv
- if [ "${TRAVIS_OS_NAME}" = "linux" ]; then ldd haproxy; fi
- if [ "${TRAVIS_OS_NAME}" = "osx" ]; then otool -L haproxy; fi
--
2.26.2
Re: No access to git.haproxy.org from Travis CI
Hi William, On Sat, Jun 13, 2020 at 03:13:06PM +0200, William Dauchy wrote: > Hi, > > On Thu, Jun 11, 2020 at 1:10 PM Willy Tarreau wrote: > > Sure but what I wanted to say was that travis seems to be the only > > point experiencing such difficulties and we don't know how it works > > nor what are the rules in place. > > I don't know whether this is related to the issue described here but I just > had: > > $ git pull --rebase > fatal: unable to access 'http://git.haproxy.org/git/haproxy.git/': The > requested URL returned error: 502 > $ git pull --rebase > error: The requested URL returned error: 502 (curl_result = 22, > http_code = 502, sha1 = f38175cf6ed4d02132e6b21cbf643f73be5ee000) > error: Unable to find f38175cf6ed4d02132e6b21cbf643f73be5ee000 under > http://git.haproxy.org/git/haproxy.git > Fetching objects: 4529, done. > Cannot obtain needed commit f38175cf6ed4d02132e6b21cbf643f73be5ee000 > while processing commit ff0e8a44a4c23ab36b6f67c4052777ac908d4211. > error: fetch failed. > > Third try was ok though :/ Thanks for the info, I'll have a look. However the issue reported about travis was a connection error, which likely indicates a routing issue. Cheers, Willy
Re: Broken SNI with crt-list for HAProxy 2.1.x after upgrade from Stretch to Buster
William, Am 13.06.20 um 16:46 schrieb Tim Düsterhus: > tune.ssl.default-dh-param 2048 solved the issue for me. > > I'd argue that this is a bug in HAProxy nonetheless, because apparently > the crt-list file is not fully parsed in case of DH parameter warnings > (not errors). In fact I can remember that some similar issue was > previously fixed. > Could this be another version of this issue: https://github.com/haproxy/haproxy/issues/483? Should I file a bug report? Best regards Tim Düsterhus
Re: Broken SNI with crt-list for HAProxy 2.1.x after upgrade from Stretch to Buster
Dear List, Am 13.06.20 um 16:11 schrieb Tim Düsterhus: > Any ideas? > Looking at the startup warnings is always a good idea: > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : > Reexecuting Master process > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : parsing > [/usr/share/haproxy//004-http.cfg:96] : 'bind :::443' : > Jun 13 14:40:52 *snip* haproxy[15815]: 'crt-list' : error processing line 1 > in file '/usr/share/haproxy/crt-list' : unable to load default 1024 bits DH > parameter for certificate '/usr/share/haproxy/ssl/*snip* > Jun 13 14:40:52 *snip* haproxy[15815]: , SSL library will use an > automatically generated DH parameter. > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : parsing > [/usr/share/haproxy//010-*snip*.cfg:7] : 'bind :::993' : > Jun 13 14:40:52 *snip* haproxy[15815]: unable to load default 1024 bits DH > parameter for certificate '/usr/share/haproxy/ssl/*snip*'. > Jun 13 14:40:52 *snip* haproxy[15815]: , SSL library will use an > automatically generated DH parameter. > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : parsing > [/usr/share/haproxy//030-*snip*.cfg:14] : 'bind :::6' : > Jun 13 14:40:52 *snip* haproxy[15815]: unable to load default 1024 bits DH > parameter for certificate '/usr/share/haproxy/ssl/*snip*'. > Jun 13 14:40:52 *snip* haproxy[15815]: , SSL library will use an > automatically generated DH parameter. > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : parsing > [/usr/share/haproxy//999-stats.cfg:2] : 'bind *:1936' : > Jun 13 14:40:52 *snip* haproxy[15815]: 'crt-list' : error processing line 1 > in file '/usr/share/haproxy/crt-list' : unable to load default 1024 bits DH > parameter for certificate '/usr/share/haproxy/ssl/*snip* > Jun 13 14:40:52 *snip* haproxy[15815]: , SSL library will use an > automatically generated DH parameter. tune.ssl.default-dh-param 2048 solved the issue for me. I'd argue that this is a bug in HAProxy nonetheless, because apparently the crt-list file is not fully parsed in case of DH parameter warnings (not errors). In fact I can remember that some similar issue was previously fixed. Best regards Tim Düsterhus
Broken SNI with crt-list for HAProxy 2.1.x after upgrade from Stretch to Buster
Dear List, I finally got around to upgrading my personal box from Debian Stretch to Debian Buster. Unfortunately after the upgrade all my HTTP hosts failed to work, because I use `strict-sni` as well as a strict `crt-list`. Software versions before the upgrade: HAProxy 2.1.7-1~bpo9+1 libssl 1.1.0l-1~deb9u1 Software versions after the upgrade: HAProxy 2.1.7-1~bpo10+1 libssl 1.1.1d-0+deb10u3 HAProxy 2.0.14-1~bpo10+1 on Buster works with SNI, but breaks my Dovecot due to the TLV stuff. Relevant bits and pieces from my config: Broken with 2.1.7 on Buster: bind :::443 ssl crt-list /usr/share/haproxy/crt-list strict-sni alpn h2,http/1.1 Working with 2.1.7 on Buster: bind :::443 ssl crt /usr/share/haproxy/ssl strict-sni alpn h2,http/1.1 My crt-list looks like this: *snip*.pem *snip*.pem [ca-file /etc/haproxy/ssl-client/*snip* verify required] With a total of 24 single-SAN certificates, 3 of those having a ca-file with verify required (the same ca-file for all of them). For the time being I used `crt` with a folder without a crt-list and killed off the domains protected by client certificate authentication. But of course this is no real solution :-) Any ideas? Best regards Tim Düsterhus
Re: No access to git.haproxy.org from Travis CI
Hi, On Thu, Jun 11, 2020 at 1:10 PM Willy Tarreau wrote: > Sure but what I wanted to say was that travis seems to be the only > point experiencing such difficulties and we don't know how it works > nor what are the rules in place. I don't know whether this is related to the issue described here but I just had: $ git pull --rebase fatal: unable to access 'http://git.haproxy.org/git/haproxy.git/': The requested URL returned error: 502 $ git pull --rebase error: The requested URL returned error: 502 (curl_result = 22, http_code = 502, sha1 = f38175cf6ed4d02132e6b21cbf643f73be5ee000) error: Unable to find f38175cf6ed4d02132e6b21cbf643f73be5ee000 under http://git.haproxy.org/git/haproxy.git Fetching objects: 4529, done. Cannot obtain needed commit f38175cf6ed4d02132e6b21cbf643f73be5ee000 while processing commit ff0e8a44a4c23ab36b6f67c4052777ac908d4211. error: fetch failed. Third try was ok though :/ -- William
