mirror of
https://git.haproxy.org/git/haproxy.git/
synced 2025-09-20 13:21:29 +02:00
BUG/MINOR: connection: streamline conn detach from lists
Over their lifetime, connections are attached to different list. These lists depends on whether connection is on frontend or backend side. Attach point members are stored via a union in struct connection. The next commit reorganizes them so that a proper frontend/backend separation is performed : commit a96f1286a75246fef6db3e615fabdef1de927d83 BUG/MINOR: connection: rearrange union list members On conn_free(), connection instance must be removed from these lists to ensure there is no use-after-free case. However code was still shaky there, despite no real issue. Indeed, <toremove_list> was detached for all connections, despite being only used on backend side only. This patch streamlines the freeing of connection. Now, <toremove_list> detach is performed in conn_backend_deinit(). Moreover, a new helper conn_frontend_deinit() is defined. It ensures that <stopping_list> detach is done. Prior it was performed individually by muxes. Note that a similar procedure is performed when the connection is reversed. Hence, conn_frontend_deinit() is now used here as well, rendering reversal from FE to BE or vice versa symmetrical. As mentionned above, no crash occured prior to this patch, but the code was fragile, in particular access to <toremove_list> for frontend connections. Thus this patch is considered as a bug fix worthy of a backport along with above mentionned patch, currently up to 3.0.
This commit is contained in:
parent
27ff7ff296
commit
687df405fe
@ -555,6 +555,18 @@ static void conn_backend_deinit(struct connection *conn)
|
|||||||
pool_free(pool_head_conn_hash_node, conn->hash_node);
|
pool_free(pool_head_conn_hash_node, conn->hash_node);
|
||||||
conn->hash_node = NULL;
|
conn->hash_node = NULL;
|
||||||
|
|
||||||
|
/* Remove from BE purge list. Necessary if conn already scheduled for
|
||||||
|
* purge but finally freed before by another code path.
|
||||||
|
*/
|
||||||
|
MT_LIST_DELETE(&conn->toremove_list);
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Ensure <conn> frontend connection is removed from its lists. This must be
|
||||||
|
* performed before freeing or reversing a connection.
|
||||||
|
*/
|
||||||
|
static void conn_frontend_deinit(struct connection *conn)
|
||||||
|
{
|
||||||
|
LIST_DEL_INIT(&conn->stopping_list);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Tries to allocate a new connection and initialized its main fields. The
|
/* Tries to allocate a new connection and initialized its main fields. The
|
||||||
@ -594,14 +606,8 @@ void conn_free(struct connection *conn)
|
|||||||
|
|
||||||
if (conn_is_back(conn))
|
if (conn_is_back(conn))
|
||||||
conn_backend_deinit(conn);
|
conn_backend_deinit(conn);
|
||||||
|
else
|
||||||
/* Remove the conn from toremove_list.
|
conn_frontend_deinit(conn);
|
||||||
*
|
|
||||||
* This is needed to prevent a double-free in case the connection was
|
|
||||||
* already scheduled from cleaning but is freed before via another
|
|
||||||
* call.
|
|
||||||
*/
|
|
||||||
MT_LIST_DELETE(&conn->toremove_list);
|
|
||||||
|
|
||||||
sockaddr_free(&conn->src);
|
sockaddr_free(&conn->src);
|
||||||
sockaddr_free(&conn->dst);
|
sockaddr_free(&conn->dst);
|
||||||
@ -2961,7 +2967,7 @@ int conn_reverse(struct connection *conn)
|
|||||||
struct server *srv = objt_server(conn->reverse.target);
|
struct server *srv = objt_server(conn->reverse.target);
|
||||||
BUG_ON(!srv);
|
BUG_ON(!srv);
|
||||||
|
|
||||||
LIST_DEL_INIT(&conn->stopping_list);
|
conn_frontend_deinit(conn);
|
||||||
|
|
||||||
if (conn_backend_init(conn))
|
if (conn_backend_init(conn))
|
||||||
return 1;
|
return 1;
|
||||||
|
@ -1412,9 +1412,6 @@ static void h1_release(struct h1c *h1c)
|
|||||||
pool_free(pool_head_h1c, h1c);
|
pool_free(pool_head_h1c, h1c);
|
||||||
|
|
||||||
if (conn) {
|
if (conn) {
|
||||||
if (!conn_is_back(conn))
|
|
||||||
LIST_DEL_INIT(&conn->stopping_list);
|
|
||||||
|
|
||||||
conn->mux = NULL;
|
conn->mux = NULL;
|
||||||
conn->ctx = NULL;
|
conn->ctx = NULL;
|
||||||
TRACE_DEVEL("freeing conn", H1_EV_H1C_END, conn);
|
TRACE_DEVEL("freeing conn", H1_EV_H1C_END, conn);
|
||||||
|
@ -1516,9 +1516,6 @@ static void h2_release(struct h2c *h2c)
|
|||||||
pool_free(pool_head_h2c, h2c);
|
pool_free(pool_head_h2c, h2c);
|
||||||
|
|
||||||
if (conn) {
|
if (conn) {
|
||||||
if (!conn_is_back(conn))
|
|
||||||
LIST_DEL_INIT(&conn->stopping_list);
|
|
||||||
|
|
||||||
conn->mux = NULL;
|
conn->mux = NULL;
|
||||||
conn->ctx = NULL;
|
conn->ctx = NULL;
|
||||||
TRACE_DEVEL("freeing conn", H2_EV_H2C_END, conn);
|
TRACE_DEVEL("freeing conn", H2_EV_H2C_END, conn);
|
||||||
|
@ -3360,8 +3360,6 @@ static void qcc_release(struct qcc *qcc)
|
|||||||
pool_free(pool_head_qcc, qcc);
|
pool_free(pool_head_qcc, qcc);
|
||||||
|
|
||||||
if (conn) {
|
if (conn) {
|
||||||
LIST_DEL_INIT(&conn->stopping_list);
|
|
||||||
|
|
||||||
conn->mux = NULL;
|
conn->mux = NULL;
|
||||||
conn->ctx = NULL;
|
conn->ctx = NULL;
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user