Skip to content

Commit

Permalink
BUG/MEDIUM: quic: fix sslconns on quic_conn alloc failure
Browse files Browse the repository at this point in the history
This commit is similar to the previous fix for QUIC actconn. It fixes
the proper accounting for global sslconns in case of allocation failure
in quic_conn().

To ensure the problem is fixed, reuse quic_release_conn_counters() to
centralize sslconns decrement.

The consequences are similar to the actconn fix : if enough memory
allocation are encountered during the process lifetime to reach
maxsslconns, all future QUIC or SSL connections opening is blocked.

This must be backported up to 2.6.
  • Loading branch information
a-denoyelle committed Nov 7, 2023
1 parent d5a9093 commit 6319920
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 16 deletions.
2 changes: 0 additions & 2 deletions include/haproxy/quic_ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ static inline void qc_free_ssl_sock_ctx(struct ssl_sock_ctx **ctx)
SSL_free((*ctx)->ssl);
pool_free(pool_head_quic_ssl_sock_ctx, *ctx);
*ctx = NULL;

_HA_ATOMIC_DEC(&global.sslconns);
}

#endif /* _HAPROXY_QUIC_SSL_H */
13 changes: 11 additions & 2 deletions src/quic_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -1162,7 +1162,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
struct quic_conn *qc = NULL;
struct listener *l = NULL;
struct quic_cc_algo *cc_algo = NULL;
unsigned int next_actconn = 0;
unsigned int next_actconn = 0, next_sslconn = 0;

TRACE_ENTER(QUIC_EV_CONN_INIT);

Expand All @@ -1173,6 +1173,12 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
goto err;
}

next_sslconn = increment_sslconn();
if (!next_sslconn) {
TRACE_STATE("sslconn reached", QUIC_EV_CONN_INIT);
goto err;
}

qc = pool_alloc(pool_head_quic_conn);
if (!qc) {
TRACE_ERROR("Could not allocate a new connection", QUIC_EV_CONN_INIT);
Expand All @@ -1182,7 +1188,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
/* Now that quic_conn instance is allocated, quic_conn_release() will
* ensure global accounting is decremented.
*/
next_actconn = 0;
next_sslconn = next_actconn = 0;

/* Initialize in priority qc members required for a safe dealloc. */
qc->nictx = NULL;
Expand Down Expand Up @@ -1382,6 +1388,8 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
*/
if (next_actconn)
_HA_ATOMIC_DEC(&actconn);
if (next_sslconn)
_HA_ATOMIC_DEC(&global.sslconns);

TRACE_LEAVE(QUIC_EV_CONN_INIT);
return NULL;
Expand Down Expand Up @@ -1519,6 +1527,7 @@ void quic_conn_release(struct quic_conn *qc)
* time with limited ressources.
*/
_HA_ATOMIC_DEC(&actconn);
_HA_ATOMIC_DEC(&global.sslconns);

TRACE_PROTO("QUIC conn. freed", QUIC_EV_CONN_FREED, qc);
leave:
Expand Down
12 changes: 0 additions & 12 deletions src/quic_rx.c
Original file line number Diff line number Diff line change
Expand Up @@ -1902,7 +1902,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
struct quic_conn *qc = NULL;
struct proxy *prx;
struct quic_counters *prx_counters;
unsigned int next_sslconn = 0;

TRACE_ENTER(QUIC_EV_CONN_LPKT);

Expand Down Expand Up @@ -1960,13 +1959,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
pkt->saddr = dgram->saddr;
ipv4 = dgram->saddr.ss_family == AF_INET;

next_sslconn = increment_sslconn();
if (!next_sslconn) {
TRACE_STATE("drop packet on sslconn reached",
QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
goto err;
}

/* Generate the first connection CID. This is derived from the client
* ODCID and address. This allows to retrieve the connection from the
* ODCID without storing it in the CID tree. This is an interesting
Expand All @@ -1985,7 +1977,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
goto err;
}

next_sslconn = 0;
/* Compute and store into the quic_conn the hash used to compute extra CIDs */
if (quic_hash64_from_cid)
qc->hash64 = quic_hash64_from_cid(conn_id->cid.data, conn_id->cid.len,
Expand Down Expand Up @@ -2036,9 +2027,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
else
HA_ATOMIC_INC(&prx_counters->dropped_pkt);

if (next_sslconn)
_HA_ATOMIC_DEC(&global.sslconns);

TRACE_LEAVE(QUIC_EV_CONN_LPKT);
return NULL;
}
Expand Down

0 comments on commit 6319920

Please sign in to comment.