BUG/MINOR: ssl: use a thread-safe sslconns increment

Each time a new SSL context is allocated, global.sslconns is
incremented. If global.maxsslconn is reached, the allocation is
cancelled.

This procedure was not entirely thread-safe due to the check and
increment operations conducted at different stage. This could lead to
global.maxsslconn slightly exceeded when several threads allocate SSL
context while sslconns is near the limit.

To fix this, use a CAS operation in a do/while loop. This code is
similar to the actconn/maxconn increment for connection.

A new function increment_sslconn() is defined for this operation. For
the moment, only SSL code is using it. However, it is expected that QUIC
will also use it to count QUIC connections as SSL ones.

This should be backported to all stable releases. Note that prior to the
2.6, sslconns was outside of global struct, so this commit should be
slightly adjusted.

diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h
index d118331f9e663a0e603ea2ec2680ca034ac40fab..02d5b023fcf63bb0651fa4125b279ea8a0324db0 100644 (file)
--- a/include/haproxy/ssl_sock.h
+++ b/include/haproxy/ssl_sock.h
@@ -113,6 +113,7 @@ int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *arg);
 int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *priv);
 #endif
 
+int increment_sslconn();
 SSL_CTX *ssl_sock_assign_generated_cert(unsigned int key, struct bind_conf *bind_conf, SSL *ssl);
 SSL_CTX *ssl_sock_get_generated_cert(unsigned int key, struct bind_conf *bind_conf);
 int ssl_sock_set_generated_cert(SSL_CTX *ctx, unsigned int key, struct bind_conf *bind_conf);

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index d2b98e2fff3a76ebca0327848665a980578ef117..8a3a1b04f13de081efeecde6d776decf7b4b5ec0 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -5698,6 +5698,27 @@ static int ssl_sock_start(struct connection *conn, void *xprt_ctx)
        return 0;
 }
 
+/* Similar to increment_actconn() but for SSL connections. */
+int increment_sslconn()
+{
+       unsigned int count, next_sslconn;
+
+       do {
+               count = global.sslconns;
+               if (global.maxsslconn && count >= global.maxsslconn) {
+                       /* maxconn reached */
+                       next_sslconn = 0;
+                       goto end;
+               }
+
+               /* try to increment sslconns */
+               next_sslconn = count + 1;
+       } while (!_HA_ATOMIC_CAS(&global.sslconns, &count, next_sslconn) && __ha_cpu_relax());
+
+ end:
+       return next_sslconn;
+}
+
 /*
  * This function is called if SSL * context is not yet allocated. The function
  * is designed to be called before any other data-layer operation and sets the
@@ -5707,6 +5728,8 @@ static int ssl_sock_start(struct connection *conn, void *xprt_ctx)
 static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
 {
        struct ssl_sock_ctx *ctx;
+       int next_sslconn = 0;
+
        /* already initialized */
        if (*xprt_ctx)
                return 0;
@@ -5734,6 +5757,12 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
        ctx->xprt_ctx = NULL;
        ctx->error_code = 0;
 
+       next_sslconn = increment_sslconn();
+       if (!next_sslconn) {
+               conn->err_code = CO_ER_SSL_TOO_MANY;
+               goto err;
+       }
+
        /* Only work with sockets for now, this should be adapted when we'll
         * add QUIC support.
         */
@@ -5743,11 +5772,6 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
                        goto err;
        }
 
-       if (global.maxsslconn && global.sslconns >= global.maxsslconn) {
-               conn->err_code = CO_ER_SSL_TOO_MANY;
-               goto err;
-       }
-
        /* If it is in client mode initiate SSL session
           in connect state otherwise accept state */
        if (objt_server(conn->target)) {
@@ -5823,7 +5847,6 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
                /* leave init state and start handshake */
                conn->flags |= CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN;
 
-               _HA_ATOMIC_INC(&global.sslconns);
                _HA_ATOMIC_INC(&global.totalsslconns);
                *xprt_ctx = ctx;
                return 0;
@@ -5856,7 +5879,6 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
                        conn->flags |= CO_FL_EARLY_SSL_HS;
 #endif
 
-               _HA_ATOMIC_INC(&global.sslconns);
                _HA_ATOMIC_INC(&global.totalsslconns);
                *xprt_ctx = ctx;
                return 0;
@@ -5864,6 +5886,8 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
        /* don't know how to handle such a target */
        conn->err_code = CO_ER_SSL_NO_TARGET;
 err:
+       if (next_sslconn)
+               _HA_ATOMIC_DEC(&global.sslconns);
        if (ctx && ctx->wait_event.tasklet)
                tasklet_free(ctx->wait_event.tasklet);
        pool_free(ssl_sock_ctx_pool, ctx);