From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 093BC1B4AA for ; Thu, 29 Nov 2018 14:24:02 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6BFF685541; Thu, 29 Nov 2018 13:24:01 +0000 (UTC) Received: from ktraynor.remote.csb (ovpn-117-230.ams2.redhat.com [10.36.117.230]) by smtp.corp.redhat.com (Postfix) with ESMTP id 27FC31001F50; Thu, 29 Nov 2018 13:23:59 +0000 (UTC) From: Kevin Traynor To: Anatoly Burakov Cc: dpdk stable Date: Thu, 29 Nov 2018 13:21:09 +0000 Message-Id: <20181129132128.7609-69-ktraynor@redhat.com> In-Reply-To: <20181129132128.7609-1-ktraynor@redhat.com> References: <20181129132128.7609-1-ktraynor@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 29 Nov 2018 13:24:01 +0000 (UTC) Subject: [dpdk-stable] patch 'ipc: fix access after async request failure' has been queued to stable release 18.08.1 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Nov 2018 13:24:02 -0000 Hi, FYI, your patch has been queued to stable release 18.08.1 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 12/08/18. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. If the code is different (ie: not only metadata diffs), due for example to a change in context or macro names, please double check it. Thanks. Kevin Traynor --- >>From 4f8eb35337f7778d64297863c8a6c5407a17f4f1 Mon Sep 17 00:00:00 2001 From: Anatoly Burakov Date: Tue, 20 Nov 2018 16:18:46 +0000 Subject: [PATCH] ipc: fix access after async request failure [ upstream commit 615fcf55d24eeb3747b3300b25f61eed8f9053ab ] Previous fix for rte_panic has moved setting of alarm before sending the message. This means that whether we send a message, the alarm would still trigger. The comment noted that cleanup would happen in the alarm handler, but that's not what actually happened - instead, in the event of failed send we freed the memory in-place, before putting the request on the queue. This works OK when the message is sent, but when sending the message fails, the alarm would still trigger with a pointer argument that points to non-existent memory, and cause memory corruption. There probably is a "proper" fix for this issue, with correct handling of sent vs. unsent requests, however it would be simpler just to sacrifice the sent request in the (extremely unlikely) event of alarm set failing. The other process would still send a response, but it will be ignored by the sender. Fixes: 45e5f49e87fb ("ipc: remove panic in async request") Signed-off-by: Anatoly Burakov --- lib/librte_eal/common/eal_common_proc.c | 40 ++++++++----------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index f65ef56c0..1c3f09aad 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -828,18 +828,17 @@ mp_request_async(const char *dst, struct rte_mp_msg *req, } - /* - * set the alarm before sending message. there are two possible error - * scenarios to consider here: - * - * - if the alarm set fails, we free the memory right there - * - if the alarm set succeeds but sending message fails, then the alarm - * will trigger and clean up the memory - * - * Even if the alarm triggers too early (i.e. immediately), we're still - * holding the lock to pending requests queue, so the interrupt thread - * will just spin until we release the lock, and either release the - * memory, or doesn't find any pending requests in the queue because we - * never added any due to send message failure. - */ + ret = send_msg(dst, req, MP_REQ); + if (ret < 0) { + RTE_LOG(ERR, EAL, "Fail to send request %s:%s\n", + dst, req->name); + ret = -1; + goto fail; + } else if (ret == 0) { + ret = 0; + goto fail; + } + param->user_reply.nb_sent++; + + /* if alarm set fails, we simply ignore the reply */ if (rte_eal_alarm_set(ts->tv_sec * 1000000 + ts->tv_nsec / 1000, async_reply_handle, pending_req) < 0) { @@ -849,19 +848,6 @@ mp_request_async(const char *dst, struct rte_mp_msg *req, goto fail; } - - ret = send_msg(dst, req, MP_REQ); - if (ret < 0) { - RTE_LOG(ERR, EAL, "Fail to send request %s:%s\n", - dst, req->name); - ret = -1; - goto fail; - } else if (ret == 0) { - ret = 0; - goto fail; - } TAILQ_INSERT_TAIL(&pending_requests.requests, pending_req, next); - param->user_reply.nb_sent++; - return 0; fail: -- 2.19.0 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2018-11-29 13:11:36.758587857 +0000 +++ 0068-ipc-fix-access-after-async-request-failure.patch 2018-11-29 13:11:34.000000000 +0000 @@ -1,8 +1,10 @@ -From 615fcf55d24eeb3747b3300b25f61eed8f9053ab Mon Sep 17 00:00:00 2001 +From 4f8eb35337f7778d64297863c8a6c5407a17f4f1 Mon Sep 17 00:00:00 2001 From: Anatoly Burakov Date: Tue, 20 Nov 2018 16:18:46 +0000 Subject: [PATCH] ipc: fix access after async request failure +[ upstream commit 615fcf55d24eeb3747b3300b25f61eed8f9053ab ] + Previous fix for rte_panic has moved setting of alarm before sending the message. This means that whether we send a message, the alarm would still trigger. The comment noted that cleanup