135 lines
5.3 KiB
Diff
135 lines
5.3 KiB
Diff
From d7be206d3570138cfadca87bb768293804629bc7 Mon Sep 17 00:00:00 2001
|
|
From: Christopher Faulet <cfaulet@haproxy.com>
|
|
Date: Tue, 28 Feb 2023 15:39:38 +0100
|
|
Subject: [PATCH] BUG/MEDIUM: connection: Clear flags when a conn is removed
|
|
from an idle list
|
|
|
|
When a connection is removed from the safe list or the idle list,
|
|
CO_FL_SAFE_LIST and CO_FL_IDLE_LIST flags must be cleared. It is performed
|
|
when the connection is reused. But not when it is moved into the
|
|
toremove_conns list. It may be an issue because the multiplexer owning the
|
|
connection may be woken up before the connection is really removed. If the
|
|
connection flags are not sanitized, it may think the connection is idle and
|
|
reinsert it in the corresponding list. From this point, we can imagine
|
|
several bugs. An UAF or a connection reused with an invalid state for
|
|
instance.
|
|
|
|
To avoid any issue, the connection flags are sanitized when an idle
|
|
connection is moved into the toremove_conns list. The same is performed at
|
|
right places in the multiplexers. Especially because the connection release
|
|
may be delayed (for h2 and fcgi connections).
|
|
|
|
This patch shoudld fix the issue #2057. It must carefully be backported as
|
|
far as 2.2. Especially on the 2.2 where the code is really different. But
|
|
some conflicts should be expected on the 2.4 too.
|
|
|
|
(cherry picked from commit 5e1b0e7bf86a300def07388df0ea7f4b3f9e68b9)
|
|
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
|
(cherry picked from commit 7902ebadb1ffbe0237ce974b950ca595894f3774)
|
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
|
|
|
Conflict: NA
|
|
Reference: https://git.haproxy.org/?p=haproxy-2.6.git;a=commit;h=d7be206d3570138cfadca87bb768293804629bc7
|
|
---
|
|
src/mux_fcgi.c | 4 +++-
|
|
src/mux_h1.c | 4 +++-
|
|
src/mux_h2.c | 7 ++++++-
|
|
src/server.c | 2 ++
|
|
4 files changed, 14 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
|
|
index c93ddd4..5a040bf 100644
|
|
--- a/src/mux_fcgi.c
|
|
+++ b/src/mux_fcgi.c
|
|
@@ -3221,8 +3221,10 @@ struct task *fcgi_timeout_task(struct task *t, void *context, unsigned int state
|
|
/* We're about to destroy the connection, so make sure nobody attempts
|
|
* to steal it from us.
|
|
*/
|
|
- if (fconn->conn->flags & CO_FL_LIST_MASK)
|
|
+ if (fconn->conn->flags & CO_FL_LIST_MASK) {
|
|
conn_delete_from_tree(&fconn->conn->hash_node->node);
|
|
+ fconn->conn->flags &= ~CO_FL_LIST_MASK;
|
|
+ }
|
|
|
|
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
}
|
|
diff --git a/src/mux_h1.c b/src/mux_h1.c
|
|
index 67d4234..a26e99c 100644
|
|
--- a/src/mux_h1.c
|
|
+++ b/src/mux_h1.c
|
|
@@ -3281,8 +3281,10 @@ struct task *h1_timeout_task(struct task *t, void *context, unsigned int state)
|
|
/* We're about to destroy the connection, so make sure nobody attempts
|
|
* to steal it from us.
|
|
*/
|
|
- if (h1c->conn->flags & CO_FL_LIST_MASK)
|
|
+ if (h1c->conn->flags & CO_FL_LIST_MASK) {
|
|
conn_delete_from_tree(&h1c->conn->hash_node->node);
|
|
+ h1c->conn->flags &= ~CO_FL_LIST_MASK;
|
|
+ }
|
|
|
|
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
}
|
|
diff --git a/src/mux_h2.c b/src/mux_h2.c
|
|
index 2215c8b..9b319c6 100644
|
|
--- a/src/mux_h2.c
|
|
+++ b/src/mux_h2.c
|
|
@@ -4147,6 +4147,7 @@ static int h2_process(struct h2c *h2c)
|
|
if (conn->flags & CO_FL_LIST_MASK) {
|
|
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
conn_delete_from_tree(&conn->hash_node->node);
|
|
+ conn->flags &= ~CO_FL_LIST_MASK;
|
|
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
}
|
|
}
|
|
@@ -4155,6 +4156,7 @@ static int h2_process(struct h2c *h2c)
|
|
if (conn->flags & CO_FL_LIST_MASK) {
|
|
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
conn_delete_from_tree(&conn->hash_node->node);
|
|
+ conn->flags &= ~CO_FL_LIST_MASK;
|
|
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
}
|
|
}
|
|
@@ -4235,8 +4237,10 @@ struct task *h2_timeout_task(struct task *t, void *context, unsigned int state)
|
|
/* We're about to destroy the connection, so make sure nobody attempts
|
|
* to steal it from us.
|
|
*/
|
|
- if (h2c->conn->flags & CO_FL_LIST_MASK)
|
|
+ if (h2c->conn->flags & CO_FL_LIST_MASK) {
|
|
conn_delete_from_tree(&h2c->conn->hash_node->node);
|
|
+ h2c->conn->flags &= ~CO_FL_LIST_MASK;
|
|
+ }
|
|
|
|
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
}
|
|
@@ -4289,6 +4293,7 @@ struct task *h2_timeout_task(struct task *t, void *context, unsigned int state)
|
|
if (h2c->conn->flags & CO_FL_LIST_MASK) {
|
|
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
conn_delete_from_tree(&h2c->conn->hash_node->node);
|
|
+ h2c->conn->flags &= ~CO_FL_LIST_MASK;
|
|
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
}
|
|
|
|
diff --git a/src/server.c b/src/server.c
|
|
index f35190d..b8e1f0a 100644
|
|
--- a/src/server.c
|
|
+++ b/src/server.c
|
|
@@ -5714,6 +5714,7 @@ static int srv_migrate_conns_to_remove(struct eb_root *idle_tree, struct mt_list
|
|
|
|
hash_node = ebmb_entry(node, struct conn_hash_node, node);
|
|
eb_delete(node);
|
|
+ hash_node->conn->flags &= ~CO_FL_LIST_MASK;
|
|
MT_LIST_APPEND(toremove_list, &hash_node->conn->toremove_list);
|
|
i++;
|
|
|
|
@@ -5771,6 +5772,7 @@ void srv_release_conn(struct server *srv, struct connection *conn)
|
|
/* Remove the connection from any tree (safe, idle or available) */
|
|
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
conn_delete_from_tree(&conn->hash_node->node);
|
|
+ conn->flags &= ~CO_FL_LIST_MASK;
|
|
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
|
}
|
|
|
|
--
|
|
2.33.0
|
|
|