BUG/MINOR: connection: rearrange union list members

A connection can be stored in several lists, thus there is several
attach points in struct connection. Depending on its proxy side, either
frontend or backend, a single connection will only access some of them
during its lifetime.

As an optimization, these attach points are organized in a union.
However, this repartition was not correctly achieved along
frontend/backend side delimitation.

Furthermore, reverse HTTP has recently been introduced. With this
feature, a connection can migrate from frontend to backend side or vice
versa. As such, it becomes even more tedious to ensure that these
members are always accessed in a safe way.

This commit rearrange these fields. First, union is now clearly splitted
between frontend and backend only elements. Next, backend elements are
initialized with conn_backend_init(), which is already used during
connection reversal on an edge endpoint. A new function
conn_frontend_init() serves to initialize the other members, called both
on connection first instantiation and on reversal on a dialer endpoint.

This model is much cleaner and should prevent any access to fields from
the wrong side.

Currently, there is no known case of wrong access in the existing code
base. However, this cleanup is considered an improvement which must be
backported up to 3.0 to remove any possible undefined behavior.
This commit is contained in:
Amaury Denoyelle 2025-08-27 14:58:59 +02:00
parent d7f6819161
commit a96f1286a7
2 changed files with 22 additions and 11 deletions

View File

@ -622,12 +622,14 @@ struct connection {
/* second cache line */
struct wait_event *subs; /* Task to wake when awaited events are ready */
union {
struct list idle_list; /* list element for idle connection in server idle list */
/* Backend connections only */
struct {
struct mt_list toremove_list; /* list element when idle connection is ready to be purged */
struct list idle_list; /* list element for idle connection in server idle list */
struct list sess_el; /* used by private connections, list elem into session */
};
union {
struct list sess_el; /* used by private backend conns, list elem into session */
struct list stopping_list; /* used by frontend conns, attach point in mux stopping list */
/* Frontend connections only */
struct list stopping_list; /* attach point in mux stopping list */
};
union conn_handle handle; /* connection handle at the socket layer */
const struct netns_entry *proxy_netns;

View File

@ -467,11 +467,6 @@ void conn_init(struct connection *conn, void *target)
conn->target = target;
conn->destroy_cb = NULL;
conn->proxy_netns = NULL;
MT_LIST_INIT(&conn->toremove_list);
if (conn_is_back(conn))
LIST_INIT(&conn->sess_el);
else
LIST_INIT(&conn->stopping_list);
LIST_INIT(&conn->tlv_list);
conn->subs = NULL;
conn->src = NULL;
@ -488,6 +483,10 @@ void conn_init(struct connection *conn, void *target)
*/
static int conn_backend_init(struct connection *conn)
{
LIST_INIT(&conn->idle_list);
LIST_INIT(&conn->sess_el);
MT_LIST_INIT(&conn->toremove_list);
if (!sockaddr_alloc(&conn->dst, 0, 0))
return 1;
@ -498,6 +497,12 @@ static int conn_backend_init(struct connection *conn)
return 0;
}
/* Initialize members used only on frontend connections. */
static void conn_frontend_init(struct connection *conn)
{
LIST_INIT(&conn->stopping_list);
}
/* Release connection elements reserved for backend side usage. It also takes
* care to detach it if linked to a session or a server instance.
*
@ -553,6 +558,9 @@ struct connection *conn_new(void *target)
return NULL;
}
}
else {
conn_frontend_init(conn);
}
return conn;
}
@ -2967,6 +2975,7 @@ int conn_reverse(struct connection *conn)
conn_backend_deinit(conn);
conn_frontend_init(conn);
conn->target = &l->obj_type;
conn->flags |= CO_FL_ACT_REVERSING;
task_wakeup(l->rx.rhttp.task, TASK_WOKEN_RES);