From 34cc55cce6b180a6c3ee3fcf70a0fd56927f240d Mon Sep 17 00:00:00 2001 From: Luc Pelletier Date: Wed, 7 Apr 2021 16:16:04 -0400 Subject: [PATCH] eal: fix race in control thread creation The creation of control threads uses a pthread barrier for synchronization. This patch fixes a race condition where the pthread barrier could get destroyed while one of the threads has not yet returned from the pthread_barrier_wait function, which could result in undefined behaviour. Fixes: 3a0d465d4c53 ("eal: fix use-after-free on control thread creation") Cc: stable@dpdk.org Signed-off-by: Luc Pelletier Acked-by: Olivier Matz Reviewed-by: Honnappa Nagarahalli --- lib/librte_eal/common/eal_common_thread.c | 49 +++++++++++++---------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c index 73a055902a..3347e91bf2 100644 --- a/lib/librte_eal/common/eal_common_thread.c +++ b/lib/librte_eal/common/eal_common_thread.c @@ -170,11 +170,19 @@ struct rte_thread_ctrl_params { void *(*start_routine)(void *); void *arg; pthread_barrier_t configured; + unsigned int refcnt; }; +static void ctrl_params_free(struct rte_thread_ctrl_params *params) +{ + if (__atomic_sub_fetch(¶ms->refcnt, 1, __ATOMIC_ACQ_REL) == 0) { + pthread_barrier_destroy(¶ms->configured); + free(params); + } +} + static void *ctrl_thread_init(void *arg) { - int ret; struct internal_config *internal_conf = eal_get_internal_configuration(); rte_cpuset_t *cpuset = &internal_conf->ctrl_cpuset; @@ -184,11 +192,8 @@ static void *ctrl_thread_init(void *arg) __rte_thread_init(rte_lcore_id(), cpuset); - ret = pthread_barrier_wait(¶ms->configured); - if (ret == PTHREAD_BARRIER_SERIAL_THREAD) { - pthread_barrier_destroy(¶ms->configured); - free(params); - } + pthread_barrier_wait(¶ms->configured); + ctrl_params_free(params); return start_routine(routine_arg); } @@ -210,15 +215,18 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name, params->start_routine = start_routine; params->arg = arg; + params->refcnt = 2; - pthread_barrier_init(¶ms->configured, NULL, 2); - - ret = pthread_create(thread, attr, ctrl_thread_init, (void *)params); + ret = pthread_barrier_init(¶ms->configured, NULL, 2); if (ret != 0) { free(params); return -ret; } + ret = pthread_create(thread, attr, ctrl_thread_init, (void *)params); + if (ret != 0) + goto fail; + if (name != NULL) { ret = rte_thread_setname(*thread, name); if (ret < 0) @@ -227,25 +235,22 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name, } ret = pthread_setaffinity_np(*thread, sizeof(*cpuset), cpuset); - if (ret) - goto fail; + if (ret != 0) + goto fail_cancel; - ret = pthread_barrier_wait(¶ms->configured); - if (ret == PTHREAD_BARRIER_SERIAL_THREAD) { - pthread_barrier_destroy(¶ms->configured); - free(params); - } + pthread_barrier_wait(¶ms->configured); + ctrl_params_free(params); return 0; -fail: - if (PTHREAD_BARRIER_SERIAL_THREAD == - pthread_barrier_wait(¶ms->configured)) { - pthread_barrier_destroy(¶ms->configured); - free(params); - } +fail_cancel: pthread_cancel(*thread); pthread_join(*thread, NULL); + +fail: + pthread_barrier_destroy(¶ms->configured); + free(params); + return -ret; } -- 2.20.1