BUG/MEDIUM: server: Add a rwlock to path parameter

Add a rwlock to control the server's path_parameter, to make sure
multiple threads don't set it at the same time, and it can't be seen in
an inconsistent state.
Also don't set the parameter every time, only set them if they have
changed, to prevent needless writes.

This does not need to be backported.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index e2036c9c76ff61b3398be1a83ce4e445b49caa80..66886190829abcae27fd1a7f8268ae7af8c41efb 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -320,6 +320,7 @@ enum renegotiate_mode {
 #define MAX_ALPN_SIZE 16
 
 struct path_parameters {
+       __decl_thread(HA_RWLOCK_T param_lock);
        char nego_alpn[MAX_ALPN_SIZE];
 };
 

diff --git a/src/backend.c b/src/backend.c
index 6412eb2b8a92a61f467a895e0f419a407a94c47c..0a5637b4b0243def3853b076d87a1b12e431007b 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -2067,10 +2067,14 @@ int connect_server(struct stream *s)
                 * that this is skipped in TCP mode as we only want mux-pt
                 * anyway.
                 */
-               if (IS_HTX_STRM(s) && srv && srv->use_ssl &&
-                   (srv->ssl_ctx.alpn_str || srv->ssl_ctx.npn_str) &&
-                   srv->path_params.nego_alpn[0] == 0)
-                       may_start_mux_now = 0;
+               if (srv) {
+                       HA_RWLOCK_RDLOCK(SERVER_LOCK, &srv->path_params.param_lock);
+                       if (IS_HTX_STRM(s) && srv->use_ssl &&
+                           (srv->ssl_ctx.alpn_str || srv->ssl_ctx.npn_str) &&
+                           srv->path_params.nego_alpn[0] == 0)
+                               may_start_mux_now = 0;
+                       HA_RWLOCK_RDUNLOCK(SERVER_LOCK, &srv->path_params.param_lock);
+               }
 #endif
 
                /* process the case where the server requires the PROXY protocol to be sent */

diff --git a/src/server.c b/src/server.c
index ba0419316a1325cb4fad2f94cce2afd509bdb2ed..7812a22d018f97bb294965373c5f68d18969515f 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -141,7 +141,9 @@ static const char *srv_op_st_chg_cause_str[] = {
 
 static void srv_reset_path_parameters(struct server *s)
 {
+       HA_RWLOCK_WRLOCK(SERVER_LOCK, &s->path_params.param_lock);
        s->path_params.nego_alpn[0] = 0;
+       HA_RWLOCK_WRUNLOCK(SERVER_LOCK, &s->path_params.param_lock);
 }
 
 const char *srv_op_st_chg_cause(enum srv_op_st_chg_cause cause)
@@ -3142,6 +3144,7 @@ struct server *new_server(struct proxy *proxy)
                sv->next = srv;
        }
 
+       HA_RWLOCK_INIT(&srv->path_params.param_lock);
        /* please don't put default server settings here, they are set in
         * proxy_preset_defaults().
         */

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 0cdd6c77e06f9dad148ec9e13fd30e7cbd230250..7e621bbe134f39d36950c734ca76c1afcc26e582 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -6825,11 +6825,15 @@ struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned int state)
                                        const char *alpn;
                                        int len;
 
-                                       if (ssl_sock_get_alpn(conn, ctx, &alpn, &len)) {
-                                               srv = objt_server(conn->target);
-                                               if (srv && len < sizeof(srv->path_params.nego_alpn)) {
+                                       srv = objt_server(conn->target);
+                                       if (srv && ssl_sock_get_alpn(conn, ctx, &alpn, &len)) {
+                                               if (len < sizeof(srv->path_params.nego_alpn) &&
+                                                   (len != strlen(srv->path_params.nego_alpn) ||
+                                                    memcmp(&srv->path_params.nego_alpn, alpn, len) != 0)) {
+                                                       HA_RWLOCK_WRLOCK(SERVER_LOCK, &srv->path_params.param_lock);
                                                        memcpy(&srv->path_params.nego_alpn, alpn, len);
                                                        srv->path_params.nego_alpn[len] = 0;
+                                                       HA_RWLOCK_WRUNLOCK(SERVER_LOCK, &srv->path_params.param_lock);
                                                }
                                        }
                                }