BUG/MINOR: connection: prevent null deref on mux cleanup task allocation

Move the code to allocate/free the mux cleanup task outside of the polling
loop. A new thread_alloc/free handler is registered for this in
connection.c.

This has the benefit to clean up the polling loop code. And as another
benefit, if the task allocation fails, the handler can report an error
to exit the haproxy process. This prevents a potential null pointer
dereferencing.

This should fix the github issue #1389.

This must be backported up to 2.4.

diff --git a/src/connection.c b/src/connection.c
index f85cae956dba8554144f824c73fe3609d309ffd2..bf8c60e4ff62094292292b58304e0a40dbf5ec97 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -1667,3 +1667,41 @@ XXH64_hash_t conn_calculate_hash(const struct conn_hash_params *params)
        hash = conn_hash_digest(buf, idx, hash_flags);
        return hash;
 }
+
+/* Handler of the task of mux_stopping_data.
+ * Called on soft-stop.
+ */
+static struct task *mux_stopping_process(struct task *t, void *ctx, unsigned int state)
+{
+       struct connection *conn, *back;
+
+       list_for_each_entry_safe(conn, back, &mux_stopping_data[tid].list, stopping_list) {
+               if (conn->mux && conn->mux->wake)
+                       conn->mux->wake(conn);
+       }
+
+       return t;
+}
+
+static int allocate_mux_cleanup(void)
+{
+       /* allocates the thread bound mux_stopping_data task */
+       mux_stopping_data[tid].task = task_new(tid_bit);
+       if (!mux_stopping_data[tid].task) {
+               ha_alert("Failed to allocate the task for connection cleanup on thread %d.\n", tid);
+               return 0;
+       }
+
+       mux_stopping_data[tid].task->process = mux_stopping_process;
+       LIST_INIT(&mux_stopping_data[tid].list);
+
+       return 1;
+}
+REGISTER_PER_THREAD_ALLOC(allocate_mux_cleanup);
+
+static int deallocate_mux_cleanup(void)
+{
+       task_destroy(mux_stopping_data[tid].task);
+       return 1;
+}
+REGISTER_PER_THREAD_FREE(deallocate_mux_cleanup);

diff --git a/src/haproxy.c b/src/haproxy.c
index 32a08441d782730ee5717059624c76e70aa1cd58..a114ff97c19d2438f1b762e81e6f2a0296883cfd 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2611,31 +2611,11 @@ __attribute__((noreturn)) void deinit_and_exit(int status)
        exit(status);
 }
 
-/* Handler of the task of mux_stopping_data.
- * Called on soft-stop.
- */
-struct task *mux_stopping_process(struct task *t, void *ctx, unsigned int state)
-{
-       struct connection *conn, *back;
-
-       list_for_each_entry_safe(conn, back, &mux_stopping_data[tid].list, stopping_list) {
-               if (conn->mux && conn->mux->wake)
-                       conn->mux->wake(conn);
-       }
-
-       return t;
-}
-
 /* Runs the polling loop */
 void run_poll_loop()
 {
        int next, wake;
 
-       /* allocates the thread bound mux_stopping_data task */
-       mux_stopping_data[tid].task = task_new(tid_bit);
-       mux_stopping_data[tid].task->process = mux_stopping_process;
-       LIST_INIT(&mux_stopping_data[tid].list);
-
        tv_update_date(0,1);
        while (1) {
                wake_expired_tasks();
@@ -2705,8 +2685,6 @@ void run_poll_loop()
 
                activity[tid].loops++;
        }
-
-       task_destroy(mux_stopping_data[tid].task);
 }
 
 static void *run_thread_poll_loop(void *data)