112 lines
4.4 KiB
Diff
112 lines
4.4 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>
|
|
---
|
|
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 31bc3ad..4981f6b 100644
|
|
--- a/src/mux_fcgi.c
|
|
+++ b/src/mux_fcgi.c
|
|
@@ -3227,8 +3227,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 754b572..56b08a7 100644
|
|
--- a/src/mux_h1.c
|
|
+++ b/src/mux_h1.c
|
|
@@ -3282,8 +3282,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 94017e9..f4cb5b1 100644
|
|
--- a/src/mux_h2.c
|
|
+++ b/src/mux_h2.c
|
|
@@ -4163,6 +4163,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);
|
|
}
|
|
}
|
|
@@ -4305,6 +4309,7 @@ do_leave:
|
|
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 cccf2f5..8a282bc 100644
|
|
--- a/src/server.c
|
|
+++ b/src/server.c
|
|
@@ -5717,6 +5717,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++;
|
|
|
|
@@ -5774,6 +5775,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);
|
|
}
|
|
|
|
--
|
|
1.7.10.4
|
|
|