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 BFB2641DAF; Wed, 1 Mar 2023 22:09:45 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AF3AB40A7E; Wed, 1 Mar 2023 22:09:45 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 6C2E040693 for ; Wed, 1 Mar 2023 22:09:44 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 3268420B9C3D; Wed, 1 Mar 2023 13:09:43 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 3268420B9C3D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1677704983; bh=v4a/EeN42mNLUDLa1G8rjBRD/JfOQtwhMhZCyvAnNbM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=MZx4OlT+rS4AlMMS51HMbsEPZRtLwwgqd0G31yYyq9Qogl444VGnBFzZ9ujKJbCWt cRZuVe4/Tw9zEIZZ6/FjAVZY59NAYtNB5VQJQllu900xst9sYs5HaLxX0PD8a/A6o8 XxooJ7A0MQhqnE24r/OxcpJhTZchgvZVVpl60icQ= From: Tyler Retzlaff To: dev@dpdk.org, david.marchand@redhat.com Cc: thomas@monjalon.net, Tyler Retzlaff Subject: [PATCH v2] eal: fix thread race in control thread creation Date: Wed, 1 Mar 2023 13:09:42 -0800 Message-Id: <1677704982-2643-1-git-send-email-roretzla@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1677518230-1194-1-git-send-email-roretzla@linux.microsoft.com> References: <1677518230-1194-1-git-send-email-roretzla@linux.microsoft.com> 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 When ctrl_thread_init transitions params->ctrl_thread_status from CTRL_THREAD_LAUNCHING the creating thread and new thread may run concurrently leading to unsynchronized access to params. This permits races for both the failure and success paths after ctrl_thread_status is stored. * params->ret may be loaded in ctrl_thread_init failure path * params->arg may be loaded in ctrl_thread_start or control_thread_start when calling start_routine. For ctrl_thread_init remove the params->ret load and just return 1 since it is only interpreted as a indicator of success / failure of ctrl_thread_init. For {ctrl,control}_thread_start store param->arg in stack allocated storage prior to calling ctrl_thread_init and use the copy when calling start_routine. For control_thread_start if ctrl_thread_init fails just return 0 instead of loading params->ret, since the value returned is unused when ctrl_thread_status is set to CTRL_THREAD_ERROR when ctrl_thread_init fails. Fixes: 878b7468eacb ("eal: add platform agnostic control thread API") Signed-off-by: Tyler Retzlaff Reviewed-by: David Marchand --- lib/eal/common/eal_common_thread.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c index edb9d4e..079a385 100644 --- a/lib/eal/common/eal_common_thread.c +++ b/lib/eal/common/eal_common_thread.c @@ -256,7 +256,7 @@ static int ctrl_thread_init(void *arg) if (params->ret != 0) { __atomic_store_n(¶ms->ctrl_thread_status, CTRL_THREAD_ERROR, __ATOMIC_RELEASE); - return params->ret; + return 1; } __atomic_store_n(¶ms->ctrl_thread_status, @@ -268,23 +268,25 @@ static int ctrl_thread_init(void *arg) static void *ctrl_thread_start(void *arg) { struct rte_thread_ctrl_params *params = arg; + void *start_arg = params->arg; void *(*start_routine)(void *) = params->u.ctrl_start_routine; if (ctrl_thread_init(arg) != 0) return NULL; - return start_routine(params->arg); + return start_routine(start_arg); } static uint32_t control_thread_start(void *arg) { struct rte_thread_ctrl_params *params = arg; + void *start_arg = params->arg; rte_thread_func start_routine = params->u.control_start_routine; if (ctrl_thread_init(arg) != 0) - return params->ret; + return 0; - return start_routine(params->arg); + return start_routine(start_arg); } int -- 1.8.3.1