Re: [PATCH 1/1] BUG/MAJOR: stick_table: Complete incomplete SEGV fix
Tim,
I've just added the following patch after finding another affected place
in smp_fetch_sc_trackers(). Fortunately this one couldn't happen, but the
checks on the code path definitely are wrong for the same reaons as in the
two other ones. Thus I prefer to make stktable_release() safe against a
NULL argument.
William, I marked it as a possible candidate for backporting to 1.8. I
think it doesn't hurt to make the code safer after these last two bugs.
Cheers,
Willy
>From 43e903553edc94bb9b33e965f37d8d218f7d1482 Mon Sep 17 00:00:00 2001
From: Willy Tarreau
Date: Wed, 27 Jun 2018 06:25:57 +0200
Subject: MINOR: stick-tables: make stktable_release() do nothing on NULL
stktable_release() has been involved in two recent crashes by being
used without enough care. Just like any free() function this one is
often called on an exit path with a possibly unsafe argument. Given
that there is another case (smp_fetch_sc_trackers()) which theorically
could call it with an unchecked NULL, though it cannot happen since
the function doesn't support being called with src_* hence cannot make
use of tmpstkctr, let's rather move the check into the function itself
to make it safer for the long term.
This patch could be backported to 1.8 as a strengthening measure.
---
src/stick_table.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/stick_table.c b/src/stick_table.c
index 8e16830..3c4e783 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -412,9 +412,11 @@ void stktable_touch_local(struct stktable *t, struct
stksess *ts, int decrefcnt)
ts->ref_cnt--;
HA_SPIN_UNLOCK(STK_TABLE_LOCK, >lock);
}
-/* Just decrease the ref_cnt of the current session */
-void stktable_release(struct stktable *t, struct stksess *ts)
+/* Just decrease the ref_cnt of the current session. Does nothing if is
NULL */
+static void stktable_release(struct stktable *t, struct stksess *ts)
{
+ if (!ts)
+ return;
HA_SPIN_LOCK(STK_TABLE_LOCK, >lock);
ts->ref_cnt--;
HA_SPIN_UNLOCK(STK_TABLE_LOCK, >lock);
@@ -875,8 +877,7 @@ static int sample_conv_in_table(const struct arg *arg_p,
struct sample *smp, voi
smp->data.type = SMP_T_BOOL;
smp->data.u.sint = !!ts;
smp->flags = SMP_F_VOL_TEST;
- if (ts)
- stktable_release(t, ts);
+ stktable_release(t, ts);
return 1;
}
@@ -2014,7 +2015,7 @@ smp_fetch_sc_tracked(const struct arg *args, struct
sample *smp, const char *kw,
smp->data.u.sint = !!stkctr;
/* release the ref count */
- if ((stkctr == ) && stkctr_entry(stkctr))
+ if ((stkctr == ))
stktable_release(stkctr->table, stkctr_entry(stkctr));
return 1;
--
1.7.12.1
Re: Haproxy 1.8 with OpenSSL 1.1.1-pre4 stops working after 1 hour
On Wed, Jun 27, 2018 at 01:44:08AM +0200, Lukas Tribus wrote: > Hey guys, > > > FYI after lots of discussions with openssl folks: > > https://github.com/openssl/openssl/issues/5330 > https://github.com/openssl/openssl/pull/6388 > https://github.com/openssl/openssl/pull/6432 > > > OpenSSL 1.1.1 will now keep the FD open by default: > > https://github.com/openssl/openssl/commit/c7504aeb640a88949dfe3146f7e0f275f517464c Wow good job Lukas! At least now we know that openssl 1.1.1 is not broken anymore! Thanks for taking care of explaining all these valid use cases there! Willy
Re: Haproxy 1.8 with OpenSSL 1.1.1-pre4 stops working after 1 hour
Hey guys, FYI after lots of discussions with openssl folks: https://github.com/openssl/openssl/issues/5330 https://github.com/openssl/openssl/pull/6388 https://github.com/openssl/openssl/pull/6432 OpenSSL 1.1.1 will now keep the FD open by default: https://github.com/openssl/openssl/commit/c7504aeb640a88949dfe3146f7e0f275f517464c cheers, lukas
Re: [PATCH 0/1] BUG/MAJOR: stick_table: Complete incomplete SEGV fix
On Tue, Jun 26, 2018 at 03:57:28PM +0200, Tim Duesterhus wrote: > attached is a patch that fixes the remaining crash and adds a > .vtc file verifying the fix. > > As the .vtc is shipped in the same patch I am not able to add the > commit message / commit id to the top of it. I hope the comment is > fine. Feel free to edit the file to suit your preferences. OK thank you for this. I applied them both together. Getting used to include test files with bug fixes is a good reflex and let's hope that over time it will become the norm. It also significantly helps to check when backporting fixes. thanks, Willy
Re: [PATCH 1/1] BUG/MAJOR: stick_table: Complete incomplete SEGV fix
On Tue, Jun 26, 2018 at 04:30:38PM +0200, Tim Düsterhus wrote: > Willy, > > Am 26.06.2018 um 16:24 schrieb Willy Tarreau: > > I *think* your patch is OK. I just hate having multiple exit points when > > refcounts are involved, we're pretty sure that in a future version we'll > > As we sent almost at the same time: I intentionally wrote it with the > new return, instead of moving it into the if, because it looks more like > sample_conv_table_conn_cur that way (see my other email). OK you're right, then let's keep it like this. Thank you Tim! Willy
Re: Haproxy health check interval value is not being respected
Hi Baptiste, Here is the haproxy configuration I have. Please let me know if you need anything else. global log 127.0.0.1 local0 nbthread 2 cpu-map auto:1/1-2 0-1 maxconn 5000 tune.bufsize 18432 tune.maxrewrite 9216 user haproxy group haproxy daemon stats socket /var/run/haproxy.sock mode 600 level admin stats timeout 2m # Wait up to 2 minutes for input tune.ssl.default-dh-param 2048 ssl-default-bind-ciphers ssl-default-bind-options no-sslv3 no-tls-tickets ssl-default-server-ciphers ssl-default-server-options no-sslv3 no-tls-tickets defaults log global option splice-auto option abortonclose timeout connect 5s timeout queue 5s timeout client 60s timeout server 60s timeout tunnel 1h timeout http-request 120s timeout check 5s option httpchk GET /graph default-server inter 10s port 80 rise 5 fall 3 cookie DO-LB insert indirect nocache maxlife 300s maxidle 300s frontend monitor bind *:50054 mode http option forwardfor monitor-uri /haproxy_test frontend tcp_80 bind 10.10.0.16:80 default_backend tcp_80_backend mode tcp backend tcp_80_backend balance leastconn mode tcp server node-359413 10.36.32.32:80 check cookie node-359413 server node-359414 10.36.32.35:80 check cookie node-359414 On Sun, Jun 17, 2018 at 6:25 AM, Baptiste wrote: > > > On Wed, Jun 13, 2018 at 6:31 PM, Adwait Gokhale > wrote: > >> Hello, >> >> I have come across an issue with use of 'inter' parameter that sets >> interval between two consecutive health checks. When configured, I found >> that health checks are twice as aggressive than what is configured. >> >> For instance, when I have haproxy with 2 backends with 'default-server >> inter 10s port 80 rise 5 fall 3' I see that health checks to every >> backend is at interval of 5 seconds instead of 10. With more than 2 >> backends, this behavior does not change. >> >> Is this a known bug or is it a misconfiguration of some sorts? Appreciate >> your help with this. >> >> Thanks, >> Adwait >> > > > Hi, > > Maybe you could share your entire configuration? > That would help a lot. > > Baptiste >
FW: Need Help!
-Original Message- From: Ray Jender [mailto:rayjen...@gmail.com] Sent: Tuesday, June 26, 2018 9:34 AM To: 'Jonathan Matthews' Subject: RE: Need Help! Thanks for the response Jonathan, Could you explain how I can set up the 4 front-ends? I am confused on how the routing would look? How HAproxy would evaluate the incoming rtmp? Thanks, Ray -Original Message- From: jonat...@jpluscplusm.com [mailto:jonat...@jpluscplusm.com] On Behalf Of Jonathan Matthews Sent: Tuesday, June 26, 2018 5:56 AM To: haproxy Cc: rayjen...@gmail.com Subject: Re: Need Help! You may not have had many replies as your email was marked as spam. You might want to address this by, amongst other things, using plain text and not HTML. On 24 June 2018 at 18:32, Ray Jender wrote: > I am sending rtmp from OBS with the streaming set to rtmp://”HAproxy > server > IP”:1935/LPC1 > frontend rtmp-in > mode tcp > acl url_LPCX path_beg -i /LPC1/ > use_backend LPC1-backend if url_LPCX > And here is the log after restarting HAproxy with mode=http: > And here is the log after restarting HAproxy with mode=tcp: You can't usefully use HTTP mode, as the traffic isn't HTTP. Haproxy doesn't speak RTMP so, in TCP mode, haproxy doesn't know how to extract path information (or anything protocol-specific) from the traffic. It can't evaluate the ACL "url_LPCX", so you can't select a backend based on it. Your best option is to have 4 frontends (or listeners) on 4 different ports, and route using that information. Jonathan
Re: [PATCH 1/1] BUG/MAJOR: stick_table: Complete incomplete SEGV fix
Willy, Am 26.06.2018 um 16:24 schrieb Willy Tarreau: > I *think* your patch is OK. I just hate having multiple exit points when > refcounts are involved, we're pretty sure that in a future version we'll As we sent almost at the same time: I intentionally wrote it with the new return, instead of moving it into the if, because it looks more like sample_conv_table_conn_cur that way (see my other email). Best regards Tim Düsterhus
Re: [PATCH 1/1] BUG/MAJOR: stick_table: Complete incomplete SEGV fix
Willy, Am 26.06.2018 um 16:17 schrieb Willy Tarreau: > On Tue, Jun 26, 2018 at 03:57:29PM +0200, Tim Duesterhus wrote: >> diff --git a/src/stick_table.c b/src/stick_table.c >> index 42946545..8e16830d 100644 >> --- a/src/stick_table.c >> +++ b/src/stick_table.c >> @@ -1596,8 +1596,10 @@ static int sample_conv_table_trackers(const struct >> arg *arg_p, struct sample *sm >> smp->data.type = SMP_T_SINT; >> smp->data.u.sint = 0; >> >> -if (ts) >> -smp->data.u.sint = ts->ref_cnt; >> +if (!ts) >> +return 1; >> + >> +smp->data.u.sint = ts->ref_cnt; >> >> stktable_release(t, ts); >> return 1; > > Well, I don't understand, this does exactly the same as the current patch > (except that your version is not thread safe), given that right now it > does this : > Note that I edited a different function: - Tim: sample_conv_table_trackers vs - Thierry: sample_conv_in_table In sample_conv_table_trackers I explicitely added the return to make the function body look like 'sample_conv_table_conn_cur' (though I forgot to add the `/* key not present */` comment, can you do so when applying?). I believe that you missed the fact that I edited a different function and thus I believe that your remaining points do not apply? Best regards Tim Düsterhus
Re: [PATCH 1/1] BUG/MAJOR: stick_table: Complete incomplete SEGV fix
On Tue, Jun 26, 2018 at 04:17:37PM +0200, Willy Tarreau wrote:
> Hi Tim,
>
> On Tue, Jun 26, 2018 at 03:57:29PM +0200, Tim Duesterhus wrote:
> > diff --git a/src/stick_table.c b/src/stick_table.c
> > index 42946545..8e16830d 100644
> > --- a/src/stick_table.c
> > +++ b/src/stick_table.c
> > @@ -1596,8 +1596,10 @@ static int sample_conv_table_trackers(const struct
> > arg *arg_p, struct sample *sm
> > smp->data.type = SMP_T_SINT;
> > smp->data.u.sint = 0;
> >
> > - if (ts)
> > - smp->data.u.sint = ts->ref_cnt;
> > + if (!ts)
> > + return 1;
> > +
> > + smp->data.u.sint = ts->ref_cnt;
> >
> > stktable_release(t, ts);
> > return 1;
>
> Well, I don't understand, this does exactly the same as the current patch
Ah I just understood, I thought you meant the current patch doesn't work
but in fact it's that another functoin is affected as well, then it
totally makes sense. This one is even more serious because it's used
much more often.
Oh and my comment about thread safety is wrong, I read too fast, I thought
you were doing ref_cnt-- here *instead* of calling stktable_release(), I'm
sorry, I'll learn to read next time.
I *think* your patch is OK. I just hate having multiple exit points when
refcounts are involved, we're pretty sure that in a future version we'll
mess up with this one when adding something else. But we can slightly
adjust it to run "if (ts) { ... }" and that will be OK.
Thank you Tim!
Willy
Re: [PATCH 1/1] BUG/MAJOR: stick_table: Complete incomplete SEGV fix
Hi Tim,
On Tue, Jun 26, 2018 at 03:57:29PM +0200, Tim Duesterhus wrote:
> diff --git a/src/stick_table.c b/src/stick_table.c
> index 42946545..8e16830d 100644
> --- a/src/stick_table.c
> +++ b/src/stick_table.c
> @@ -1596,8 +1596,10 @@ static int sample_conv_table_trackers(const struct arg
> *arg_p, struct sample *sm
> smp->data.type = SMP_T_SINT;
> smp->data.u.sint = 0;
>
> - if (ts)
> - smp->data.u.sint = ts->ref_cnt;
> + if (!ts)
> + return 1;
> +
> + smp->data.u.sint = ts->ref_cnt;
>
> stktable_release(t, ts);
> return 1;
Well, I don't understand, this does exactly the same as the current patch
(except that your version is not thread safe), given that right now it
does this :
/* Just decrease the ref_cnt of the current session */
void stktable_release(struct stktable *t, struct stksess *ts)
{
HA_SPIN_LOCK(STK_TABLE_LOCK, >lock);
ts->ref_cnt--;
HA_SPIN_UNLOCK(STK_TABLE_LOCK, >lock);
}
...
smp->data.type = SMP_T_BOOL;
smp->data.u.sint = !!ts;
smp->flags = SMP_F_VOL_TEST;
if (ts)
stktable_release(t, ts);
...
So that's in fact :
smp->data.type = SMP_T_BOOL;
smp->data.u.sint = !!ts;
smp->flags = SMP_F_VOL_TEST;
if (ts) {
HA_SPIN_LOCK(STK_TABLE_LOCK, >lock);
ts->ref_cnt--;
HA_SPIN_UNLOCK(STK_TABLE_LOCK, >lock);
}
Thus, if ts is NULL, we do return immediately with value zero, and
if it's not NULL, we properly decrement the refcount.
Given that your patch is applied on top of the unfixed version, I
suspect that you did a manipulation error and that you tested only
this unfixed version instead.
Could you please double-check ?
Thanks,
Willy
[PATCH 0/1] BUG/MAJOR: stick_table: Complete incomplete SEGV fix
Hi attached is a patch that fixes the remaining crash and adds a .vtc file verifying the fix. As the .vtc is shipped in the same patch I am not able to add the commit message / commit id to the top of it. I hope the comment is fine. Feel free to edit the file to suit your preferences. Best regards Tim Duesterhus (1): BUG/MAJOR: stick_table: Complete incomplete SEGV fix reg-tests/stick-table/h0.vtc | 30 ++ src/stick_table.c| 6 -- 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 reg-tests/stick-table/h0.vtc -- 2.18.0
[PATCH 1/1] BUG/MAJOR: stick_table: Complete incomplete SEGV fix
This commit completes the incomplete segmentation fault fix
in commit ac1f3ed64b58bd178865c6f2cc8f6f306d9e1e15.
Likewise it must be backported to haproxy 1.8.
---
reg-tests/stick-table/h0.vtc | 30 ++
src/stick_table.c| 6 --
2 files changed, 34 insertions(+), 2 deletions(-)
create mode 100644 reg-tests/stick-table/h0.vtc
diff --git a/reg-tests/stick-table/h0.vtc b/reg-tests/stick-table/h0.vtc
new file mode 100644
index ..1071bc04
--- /dev/null
+++ b/reg-tests/stick-table/h0.vtc
@@ -0,0 +1,30 @@
+# Shipped with the commit fixing the bug.
+
+varnishtest "Stick Table: Crash when accessing unknown key."
+feature ignore_unknown_macro
+
+server s0 {
+ rxreq
+ txresp
+} -start
+
+haproxy h0 -conf {
+ defaults
+ timeout connect 5000ms
+ timeout client 5ms
+ timeout server 5ms
+
+ frontend test
+ mode http
+ bind "fd@${fe1}"
+ stick-table type ip size 1m expire 1h store gpc0
+ http-request deny if { src,table_trackers(test) eq 1 }
+ http-request deny if { src,in_table(test) }
+ http-request deny deny_status 200
+} -start
+
+client c0 -connect ${h0_fe1_sock} {
+ txreq -url "/"
+ rxresp
+ expect resp.status == 200
+} -run
diff --git a/src/stick_table.c b/src/stick_table.c
index 42946545..8e16830d 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -1596,8 +1596,10 @@ static int sample_conv_table_trackers(const struct arg
*arg_p, struct sample *sm
smp->data.type = SMP_T_SINT;
smp->data.u.sint = 0;
- if (ts)
- smp->data.u.sint = ts->ref_cnt;
+ if (!ts)
+ return 1;
+
+ smp->data.u.sint = ts->ref_cnt;
stktable_release(t, ts);
return 1;
--
2.18.0
Re: [Patch] Re: Segfault with haproxy 1.8.10
Hi Am 26.06.2018 um 13:56 schrieb Willy Tarreau: > Your patch is obviously good, I've just merged it. > Should sample_conv_table_trackers also be updated? It also checks whether `ts` is valid, before accessing it, but unconditionally calls stktable_release later on. Best regards Tim Düsterhus
Re: Fails to build HAProxy 1.8.10 without USE_THREAD
On Sun, Jun 24, 2018 at 10:05:48AM +, Zero King wrote:
> On Sun, Jun 24, 2018 at 11:45:35AM +0200, William Lallemand wrote:
> >On Sun, Jun 24, 2018 at 09:35:11AM +, Zero King wrote:
> >> On Sun, Jun 24, 2018 at 09:42:54AM +0200, William Lallemand wrote:
> >> >On Sun, Jun 24, 2018 at 02:30:57AM +, Zero King wrote:
> >> >> Hi,
> >> >>
> >> >> I tried to update haproxy to 1.8.10 in MacPorts, but it fails to build
> >> >> from source with the following error (without USE_THREAD):
> >> >>
> >> >> /usr/bin/clang -arch x86_64 -Iinclude -Iebtree -Wall -O2 -g
> >> >> -fno-strict-aliasing -Wdeclaration-after-statement -fwrapv
> >> >> -fno-strict-overflow -Wno-address-of-packed-member
> >> >> -Wno-null-dereference -Wno-unused-label -DTPROXY
> >> >> -DCONFIG_HAP_CRYPT -DUSE_ZLIB -DENABLE_POLL -DENABLE_KQUEUE
> >> >> -DCONFIG_REGPARM=3 -DUSE_OPENSSL -DUSE_PCRE -I/opt/local/include
> >> >> -DCONFIG_HAPROXY_VERSION=\"1.8.10-ec17d7a\"
> >> >> -DCONFIG_HAPROXY_DATE=\"2018/06/22\" -c -o src/log.o src/log.c
> >> >> src/haproxy.c:2475:16: error: cannot take the address of an rvalue of
> >> >> type 'unsigned long'
> >> >> HA_ATOMIC_AND(_threads_mask, ~tid_bit);
> >> >> ^
> >> >> include/common/hathreads.h:41:42: note: expanded from macro
> >> >> 'HA_ATOMIC_AND'
> >> >> #define HA_ATOMIC_AND(val, flags)({*(val) &= (flags);})
> >> >> ^~~
> >> >> 1 error generated.
> >> >>
> >> >
> >> >
> >> >Thanks for your report, there a patch that should solve this issue.
> >>
> >> Thanks for the patch, I applied it in OS X < 10.7 [1], which doesn't
> >> support USE_THREAD, and build passed.
> >>
> >> [1]:
> >> https://github.com/macports/macports-ports/commit/a8f179575f70f0d21dfbaef7a9003cbcb43b59bb
> >>
> >
> >I don't own an OS X machine so that's difficult for me to test, but what is
> >the
> >reason USE_THREAD is not supported in < 10.7?
>
> Lack of thread-local storage support, see
> https://github.com/macports/macports-ports/commit/35ea8581e16eb3cc92f03ec6bc3f21959d0a44e4#diff-ded0796a87dca23e1462f6820eaaa86dR245.
> I don't have an old system myself so I went for the easy way and
> disabled threads.
>
> /usr/bin/gcc-4.2 -arch x86_64 -Iinclude -Iebtree -Wall -O2 -g
> -fno-strict-aliasing -Wdeclaration-after-statement -fwrapv
> -fno-strict-overflow-Wno-unused-label -DTPROXY -DCONFIG_HAP_CRYPT
> -DUSE_ZLIB -DENABLE_POLL -DENABLE_KQUEUE -DCONFIG_REGPARM=3 -DUSE_THREAD
> -DUSE_OPENSSL -DUSE_PCRE -I/opt/local/include
> -DCONFIG_HAPROXY_VERSION=\"1.8.10-ec17d7a\"
> -DCONFIG_HAPROXY_DATE=\"2018/06/22\" -c -o ebtree/ebmbtree.o ebtree/ebmbtree.c
> In file included from include/common/memory.h:32,
> from include/common/chunk.h:29,
> from include/common/standard.h:36,
> from include/common/ticks.h:56,
> from src/ev_kqueue.c:22:
> include/common/hathreads.h:28: error: thread-local storage not supported for
> this target
> include/common/hathreads.h:29: error: thread-local storage not supported for
> this target
> In file included from include/common/memory.h:32,
> from include/common/chunk.h:29,
> from include/common/standard.h:36,
> from include/common/ticks.h:56,
> from src/ev_poll.c:22:
> include/common/hathreads.h:28: error: thread-local storage not supported for
> this target
> include/common/hathreads.h:29: error: thread-local storage not supported for
> this target
> In file included from include/common/ticks.h:56,
> from src/ev_kqueue.c:22:
> include/common/standard.h:96: error: thread-local storage not supported for
> this target
> include/common/standard.h:111: error: thread-local storage not supported for
> this target
> In file included from include/common/ticks.h:56,
> from src/ev_poll.c:22:
> include/common/standard.h:96: error: thread-local storage not supported for
> this target
> include/common/standard.h:111: error: thread-local storage not supported for
> this target
>
Thanks for the informations, you can build directly the 1.8.11 version which
includes that patch now :-)
--
William Lallemand
[ANNOUNCE] haproxy-1.8.11
Hi, HAProxy 1.8.11 was released on 2018/06/26. It added 2 new commits after version 1.8.10. This version appeared earlier than expected because of a regression in 1.8.10. A fix in the stick table was causing a segfault which is triggered when trying to lookup a key that does not exist. It also fixes the build without threads which was broken once again, I'll build without threads before a release to be sure it does not happened anymore :-) People are encouraged to update directly in 1.8.11 if they didn't update to 1.8.10. Please find the usual URLs below : Site index : http://www.haproxy.org/ Discourse: http://discourse.haproxy.org/ Sources : http://www.haproxy.org/download/1.8/src/ Git repository : http://git.haproxy.org/git/haproxy-1.8.git/ Git Web browsing : http://git.haproxy.org/?p=haproxy-1.8.git Changelog: http://www.haproxy.org/download/1.8/src/CHANGELOG Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/ --- Complete changelog : Thierry FOURNIER (1): BUG/MAJOR: Stick-tables crash with segfault when the key is not in the stick-table William Lallemand (1): BUG/BUILD: threads: unbreak build without threads --- -- William Lallemand
Re: Fails to build HAProxy 1.8.10 without USE_THREAD
On Sun, Jun 24, 2018 at 11:16:05AM +0200, William Lallemand wrote:
> From 081e7f64b1f2ff882be5eb1cb13e44fd2587e806 Mon Sep 17 00:00:00 2001
> From: William Lallemand
> Date: Sun, 24 Jun 2018 09:37:03 +0200
> Subject: [PATCH] BUG/BUILD: threads: unbreak build without threads
>
> The build without threads was once again broken.
>
> This issue was introduced in commit ba86c6c ("MINOR: threads: Be sure to
> remove threads from all_threads_mask on exit").
>
> This is exactly the same problem as last time it happened, because of
> all_threads_mask not being defined with USE_THREAD=
>
> This must be backported in 1.8
Just merged, thanks.
Willy
Re: [Patch] Re: Segfault with haproxy 1.8.10
On Tue, Jun 26, 2018 at 02:04:21PM +0200, Thierry Fournier wrote: > >> William, The backport in 1.8 stable is trivial. > > > > William / Christopher, given that 1.8.10 is very recent and few people > > had the time to upgrade, I think it would make sense to issue 1.8.11 > > to limit the risks. I don't like such bugs which crash at runtime > > while you're drinking coffee after the upgrade. > > Agreed. > Arggg ... I just created my own maintenance branch with the patch for my > customers and own projets :-) > > Thierry Thanks Thierry! Backported in 1.8, I will issue a 1.8.11 today or tomorrow. -- William Lallemand
Re: [Patch] Re: Segfault with haproxy 1.8.10
> On 26 Jun 2018, at 13:56, Willy Tarreau wrote: > > Hi Thierry, > > On Tue, Jun 26, 2018 at 10:20:38AM +0200, thierry.fourn...@arpalert.org wrote: >> BR,Hi, >> >> I found the bug. The segfault is unavoidable, because is trigged if an >> entry doesn't exists in the stick-tables. At the start, the stick table >> is empty. >> >> It is a regression introduced in 1.8.10 by this patch: >> >> BUG/MEDIUM: stick-tables: Decrement ref_cnt in table_* converters >> commit d7bd88009d88dd413e01bc0baa90d6662a3d7718 >> Author: Daniel Corbett >> Date: Sun May 27 09:47:12 2018 -0400 > > Oh crap :-( I noticed the memory leaks in the first version but I did > not notice this one, which is pretty visible nonetheless given the test > on !!ts 2 lines above :-( It happens ... >> I join a patch. >> >> Daniel, could you check the compliance with your original patch ? > > Your patch is obviously good, I've just merged it. great. >> William, The backport in 1.8 stable is trivial. > > William / Christopher, given that 1.8.10 is very recent and few people > had the time to upgrade, I think it would make sense to issue 1.8.11 > to limit the risks. I don't like such bugs which crash at runtime > while you're drinking coffee after the upgrade. Arggg ... I just created my own maintenance branch with the patch for my customers and own projets :-) Thierry
Re: [Patch] Re: Segfault with haproxy 1.8.10
Hi Thierry, On Tue, Jun 26, 2018 at 10:20:38AM +0200, thierry.fourn...@arpalert.org wrote: > BR,Hi, > > I found the bug. The segfault is unavoidable, because is trigged if an > entry doesn't exists in the stick-tables. At the start, the stick table > is empty. > > It is a regression introduced in 1.8.10 by this patch: > >BUG/MEDIUM: stick-tables: Decrement ref_cnt in table_* converters >commit d7bd88009d88dd413e01bc0baa90d6662a3d7718 >Author: Daniel Corbett >Date: Sun May 27 09:47:12 2018 -0400 Oh crap :-( I noticed the memory leaks in the first version but I did not notice this one, which is pretty visible nonetheless given the test on !!ts 2 lines above :-( > I join a patch. > > Daniel, could you check the compliance with your original patch ? Your patch is obviously good, I've just merged it. > William, The backport in 1.8 stable is trivial. William / Christopher, given that 1.8.10 is very recent and few people had the time to upgrade, I think it would make sense to issue 1.8.11 to limit the risks. I don't like such bugs which crash at runtime while you're drinking coffee after the upgrade. Thanks Thierry! Willy
Re: Need Help!
You may not have had many replies as your email was marked as spam. You might want to address this by, amongst other things, using plain text and not HTML. On 24 June 2018 at 18:32, Ray Jender wrote: > I am sending rtmp from OBS with the streaming set to rtmp://”HAproxy server > IP”:1935/LPC1 > frontend rtmp-in > mode tcp > acl url_LPCX path_beg -i /LPC1/ > use_backend LPC1-backend if url_LPCX > And here is the log after restarting HAproxy with mode=http: > And here is the log after restarting HAproxy with mode=tcp: You can't usefully use HTTP mode, as the traffic isn't HTTP. Haproxy doesn't speak RTMP so, in TCP mode, haproxy doesn't know how to extract path information (or anything protocol-specific) from the traffic. It can't evaluate the ACL "url_LPCX", so you can't select a backend based on it. Your best option is to have 4 frontends (or listeners) on 4 different ports, and route using that information. Jonathan
[Patch] Re: Segfault with haproxy 1.8.10
BR,Hi,
I found the bug. The segfault is unavoidable, because is trigged if an
entry doesn't exists in the stick-tables. At the start, the stick table
is empty.
It is a regression introduced in 1.8.10 by this patch:
BUG/MEDIUM: stick-tables: Decrement ref_cnt in table_* converters
commit d7bd88009d88dd413e01bc0baa90d6662a3d7718
Author: Daniel Corbett
Date: Sun May 27 09:47:12 2018 -0400
I join a patch.
Daniel, could you check the compliance with your original patch ?
William, The backport in 1.8 stable is trivial.
BR,
Thierry
On Mon, 25 Jun 2018 23:03:37 +0200
Willy Tarreau wrote:
> On Mon, Jun 25, 2018 at 10:45:51PM +0200, Thierry Fournier wrote:
> > Just for information, If someone is working on this bug, I think
> > that I found the origin of the crash. I check impact and the
> > validity of the patch, and them I submit a patch
>
> Ah cool, thank you, we'll have a fairly busy week and I didn't expect
> to have the time to look at this crash this week :-(
>
> Cheers,
> Willy
>
>From 92622852bccce39afbc63320ee7cad4df0586388 Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER
Date: Mon, 25 Jun 2018 22:35:20 +0200
Subject: [PATCH] BUG/MAJOR: Stick-tables crash with segfault when the key is
not in the stick-table
When a lookup is done on a key not present in the stick-table the "st"
pointer is NULL and it is used to return the converter result, but it
is used untested with stktable_release().
This regression was introduced in 1.8.10 here:
BUG/MEDIUM: stick-tables: Decrement ref_cnt in table_* converters
commit d7bd88009d88dd413e01bc0baa90d6662a3d7718
Author: Daniel Corbett
Date: Sun May 27 09:47:12 2018 -0400
Minimal conf for reproducong the problem:
frontend test
mode http
stick-table type ip size 1m expire 1h store gpc0
bind *:8080
http-request redirect location /a if { src,in_table(test) }
The segfault is triggered using:
curl -i http://127.0.0.1:8080/
This patch must be backported in 1.8
---
src/stick_table.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/stick_table.c b/src/stick_table.c
index 101a4e253..429465455 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -875,7 +875,8 @@ static int sample_conv_in_table(const struct arg *arg_p, struct sample *smp, voi
smp->data.type = SMP_T_BOOL;
smp->data.u.sint = !!ts;
smp->flags = SMP_F_VOL_TEST;
- stktable_release(t, ts);
+ if (ts)
+ stktable_release(t, ts);
return 1;
}
--
2.16.3
