From 4837293ca0a1cc8c55ee9b84e66ee30bc755c291 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Thu, 16 Sep 2021 12:15:12 +0200 Subject: [PATCH] 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. --- src/connection.c | 38 ++++++++++++++++++++++++++++++++++++++ src/haproxy.c | 22 ---------------------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/connection.c b/src/connection.c index f85cae956..bf8c60e4f 100644 --- 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 32a08441d..a114ff97c 100644 --- 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)