eal: fix possible UB on creation of ctrl thread
Checks
Commit Message
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: jianfeng.tan@intel.com
Cc: stable@dpdk.org
Signed-off-by: Luc Pelletier <lucp.at.work@gmail.com>
---
lib/librte_eal/common/eal_common_thread.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
Comments
Hi Olivier,
Hi Honnappa,
Thanks for your input Honnappa. I've made the changes to
completely remove the barrier. However, I have not moved the
call to pthread_setaffinity_np to the control thread; I
think we still want to report the result of this function
to the caller of rte_ctrl_thread_create and doing so from
ctrl_thread_init would be a lot trickier.
@@ -170,6 +170,7 @@ struct rte_thread_ctrl_params {
void *(*start_routine)(void *);
void *arg;
pthread_barrier_t configured;
+ bool barrier_in_use;
};
static void *ctrl_thread_init(void *arg)
@@ -186,9 +187,13 @@ static void *ctrl_thread_init(void *arg)
ret = pthread_barrier_wait(¶ms->configured);
if (ret == PTHREAD_BARRIER_SERIAL_THREAD) {
+ while (__atomic_load_n(¶ms->barrier_in_use,
+ __ATOMIC_ACQUIRE))
+ sched_yield();
pthread_barrier_destroy(¶ms->configured);
free(params);
- }
+ } else
+ __atomic_store_n(¶ms->barrier_in_use, 0, __ATOMIC_RELEASE);
return start_routine(routine_arg);
}
@@ -210,6 +215,7 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
params->start_routine = start_routine;
params->arg = arg;
+ params->barrier_in_use = 1;
pthread_barrier_init(¶ms->configured, NULL, 2);
@@ -232,18 +238,26 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
ret = pthread_barrier_wait(¶ms->configured);
if (ret == PTHREAD_BARRIER_SERIAL_THREAD) {
+ while (__atomic_load_n(¶ms->barrier_in_use,
+ __ATOMIC_ACQUIRE))
+ sched_yield();
pthread_barrier_destroy(¶ms->configured);
free(params);
- }
+ } else
+ __atomic_store_n(¶ms->barrier_in_use, 0, __ATOMIC_RELEASE);
return 0;
fail:
if (PTHREAD_BARRIER_SERIAL_THREAD ==
pthread_barrier_wait(¶ms->configured)) {
+ while (__atomic_load_n(¶ms->barrier_in_use,
+ __ATOMIC_ACQUIRE))
+ sched_yield();
pthread_barrier_destroy(¶ms->configured);
free(params);
- }
+ } else
+ __atomic_store_n(¶ms->barrier_in_use, 0, __ATOMIC_RELEASE);
pthread_cancel(*thread);
pthread_join(*thread, NULL);
return -ret;