From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id EBDC0A0A0F; Wed, 7 Apr 2021 22:26:22 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A88B9140F81; Wed, 7 Apr 2021 22:26:22 +0200 (CEST) Received: from mail-qt1-f182.google.com (mail-qt1-f182.google.com [209.85.160.182]) by mails.dpdk.org (Postfix) with ESMTP id 4E0C6140F7F; Wed, 7 Apr 2021 22:26:21 +0200 (CEST) Received: by mail-qt1-f182.google.com with SMTP id x9so14751547qto.8; Wed, 07 Apr 2021 13:26:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=5vS+BVSyrRgO5Q6IVHgdfSLPFiao1qp6vZGA1gmn7a0=; b=lDu74IngwDYMq3AITX1+VZKcZef3zlX8ba6E+/o0fPyFdIy95dHci4QT5s7PYCg92d 6zOSacQL0XTbSdnG8FauQCpphtlaEa73lHEkKbqI1pzum1mdOsLC8wDDG74CaVNxE4Kt MPQk2cBuqtgxq6JNFtFIDm6w4UYP2/tQeDMegTdFBgPTSuxm3mTNTGH2R/9WM+0JpKRQ 64IVivvkMuPZx3X6+dtjkQo2cUFL26JOFMpFWAVFrjXUtPo9scRazVXR1fe6knZcjcOS uWyUllgbOGuugkf/Hxj49XCHdx9Jdcah/nwt0aP+fpF1Y/euLVZwelxbEYADCujlq8yr EJuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=5vS+BVSyrRgO5Q6IVHgdfSLPFiao1qp6vZGA1gmn7a0=; b=L8Dpe8Z2UqBnTX87JkMQnWnX62ZtLf/OKqtYd8RuRafHCGymQJGyNqv7muKJ+5JjKV 31rK1EYVHayVHX0kaXNE+MbRdrq68vaqVuwqsIKLSha8BCaIL4HZXBM8QxD1fnWBlUlB 5UlRwv1D4bUBbAp8vO9WAy6REn9xWVdQK78OkyDokeHiUib7kGz59Q5IylQgzvtyy+gg hKvr9SgJDtXRessGcIz+HMfDBT5+FQ1ZBomVs8GhiCQCWQcah3ZhpkRB286MqFgL3h38 drA/HrQ2L8CO/nm7DSmBLwq6ynFeokN82i40vR/qm+UqgdNt899TTtBMd3W1UiyOet6W NDBg== X-Gm-Message-State: AOAM530CvVw2CH7CmK5M4z2XIH0xTyYDeGWyMkjO1WPGyS+G0wHphORj 7hbU7H/EgsUH+bj2DCCH/ks= X-Google-Smtp-Source: ABdhPJy/wX5Hz+o70Ya65+qhvbluBva51sZBjpRGjFe9KGk29oTGw05AafC1vThr/yjtf8M1RmmN/A== X-Received: by 2002:a05:622a:15d4:: with SMTP id d20mr4303144qty.85.1617827180791; Wed, 07 Apr 2021 13:26:20 -0700 (PDT) Received: from localhost.localdomain (bras-base-hullpq2034w-grc-18-74-15-213-92.dsl.bell.ca. [74.15.213.92]) by smtp.gmail.com with ESMTPSA id 7sm19532060qkm.64.2021.04.07.13.26.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 07 Apr 2021 13:26:20 -0700 (PDT) From: Luc Pelletier To: olivier.matz@6wind.com, jianfeng.tan@intel.com, Honnappa.Nagarahalli@arm.com Cc: dev@dpdk.org, Luc Pelletier , stable@dpdk.org Date: Wed, 7 Apr 2021 16:16:04 -0400 Message-Id: <20210407201603.149234-1-lucp.at.work@gmail.com> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH 1/2] eal: fix race in ctrl thread creation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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: 3a0d465 ("eal: fix use-after-free on control thread creation") Cc: jianfeng.tan@intel.com Cc: stable@dpdk.org Signed-off-by: Luc Pelletier --- Hi Olivier, Hi Honnappa, As discussed, I've split the changes into 2 patches. This first commit fixes the race condition but leaves in the pthread_cancel call. 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 73a055902..3347e91bf 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.25.1