On 3/24/20 7:01 PM, William Lallemand wrote:
On Tue, Mar 24, 2020 at 05:41:48PM +0100, Olivier D wrote:

With latest haproxy 2.0, you can generate a simple segfault with only
configuration test (haproxy -f test.cfg -c)

Config content :
     mode http

backend test
   stick-table type ip size 10k expire 1h store http_req_rate(1h) peers

peers mypeers
     peer toto
Running :
[WARNING] 083/173758 (2599) : Removing incomplete section 'peers mypeers'
(no peer named 'myhostname.localhost').
Segmentation fault

peers_register_table (peers=0xc8a110, table=table@entry=0xc8c6f0) at
(gdb) bt
#0  peers_register_table (peers=0xc8a110, table=table@entry=0xc8c6f0) at
#1  0x0000000000522bec in stktable_init (t=0xc8c6f0) at
#2  0x000000000050fa67 in check_config_validity () at src/cfgparse.c:4048
#3  0x0000000000505311 in init (argc=<optimized out>, argc@entry=4,
argv=<optimized out>, argv@entry=0x7fffffffe848) at src/haproxy.c:1760
#4  0x000000000046475f in main (argc=4, argv=0x7fffffffe848) at

I know my peer entry is not correct (my hostname is not "toto") but we
should not end with a segfault ^^

I agree. It looks like this is the consequence of the peer being free'd
if you don't reference the local peer in the section. It should still
start in your case by specifying -L toto on the command line.

I think the peer pointer is still referenced in the stick-table which
tries to use it during its initialization. So if we want to free the
peer we need to remove its reference too.


I think you are true, but I did not manage to reproduce this issue. But I think you are correct about the reference to the peers section which may be found in stktable structs.

Here is a patch which may fix this issue.

Olivier, please could you give it a try?


>From 9b4ea422ceb3a06cb2692d56475dfa18ec02bbf1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= <flecai...@haproxy.com>
Date: Tue, 24 Mar 2020 20:08:30 +0100
Subject: [PATCH] BUG/MINOR: peers: Use after free of "peers" section.

When a "peers" section has not any local peer, it is removed of the list
of "peers" sections by check_config_validity(). But a stick-table which
refers to a "peers" section stores a pointer to this peers section.
These pointer must be reset to NULL value for each stick-table refering to
such a "peers" section to prevent stktable_init() to start the peers frontend
attached to the peers section dereferencing the invalid pointer.

Furthemore this patch stops the peers frontend as this is done for other
configurations invalidated by check_config_validity().

Thank you to Olivier D for having reported this issue with such a
simple configuration file which made haproxy crash when started with
-c option for configuration file validation.

    mode http

  peers mypeers
    peer toto

  backend test
    stick-table type ip size 10k expire 1h store http_req_rate(1h) peers mypeers

Must be backported to 2.1 and 2.0.
 src/cfgparse.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index eaa853f6e..2c8008312 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -3925,6 +3925,7 @@ out_uri_auth_compat:
 		last = &cfg_peers;
 		while (*last) {
+			struct stktable *t;
 			curpeers = *last;
 			if (curpeers->state == PR_STSTOPPED) {
@@ -3936,6 +3937,9 @@ out_uri_auth_compat:
 			else if (!curpeers->peers_fe || !curpeers->peers_fe->id) {
 				ha_warning("Removing incomplete section 'peers %s' (no peer named '%s').\n",
 					   curpeers->id, localpeer);
+				if (curpeers->peers_fe)
+					stop_proxy(curpeers->peers_fe);
+				curpeers->peers_fe = NULL;
 			else if (atleast2(curpeers->peers_fe->bind_proc)) {
 				/* either it's totally stopped or too much used */
@@ -3998,6 +4002,11 @@ out_uri_auth_compat:
 			curpeers = curpeers->next;
+			/* Reset any refereance to this peers section in the list of stick-tables */
+			for (t = stktables_list; t; t = t->next) {
+				if (t->peers.p && t->peers.p == *last)
+					t->peers.p = NULL;
+			}
 			*last = curpeers;

Reply via email to