From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 2346DA051C
	for <public@inbox.dpdk.org>; Tue, 11 Feb 2020 12:22:36 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 166272B9C;
	Tue, 11 Feb 2020 12:22:36 +0100 (CET)
Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com
 [209.85.221.41]) by dpdk.org (Postfix) with ESMTP id EE5732B9C
 for <stable@dpdk.org>; Tue, 11 Feb 2020 12:22:34 +0100 (CET)
Received: by mail-wr1-f41.google.com with SMTP id g3so10775373wrs.12
 for <stable@dpdk.org>; Tue, 11 Feb 2020 03:22:34 -0800 (PST)
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=3JXxhzsGnAUj4HzFLGOTaDr+Q2JWqQYHHptU5Af3aRg=;
 b=o1NlOEdIatqQDd9YdXDwbEgB5pqJpQ/0Ji8tq3KjKDN3aZmQEZcMOEpADZFjwXcYoI
 50BHHxVv+VeHdvaSMYxRXLIJ3JlOllEc6rPRpnViOAI3MOxPQ5UdV6oj7IyrCEKOb2m6
 D7aLSmv57xfFvmyRH4uD+5Yyg63ZpBoOPf6dx84nLEoTtN3j2t66aPXpcuh/FM6XHZEE
 XixYbXmxxNG9ZLlAT+8JaJpXj8r/qK/cTH4l/moHJk8U1EL5luWvxmyblpdzYTUR82zZ
 BHilMrsWij8VJsaVkjbJwPcesN6Kl1BLw36eHIHR+LnaD2CJ9Rr2jzA1oVN7kQX77ZKs
 VWSQ==
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=3JXxhzsGnAUj4HzFLGOTaDr+Q2JWqQYHHptU5Af3aRg=;
 b=sXwqr43Mkt8QUaDyDSRaB8zQ0JKqf7gVaREa3ZlV3Q1muUQKZVjDpvjUsihhf5Z94O
 BfxU669wa22btpbLPPF6XXwAUrWYr62P4xV1ZbjPbmWLA4txnxPBSGpFDde4GD/o+mLu
 gIsb5KAZ7FG7FueqF8A3TiuQHk/dOofi3aG7+6aqmLB+2CElV9SRYe6wEFQX6qwhAduB
 +bmtYyVOEldRt/NYX003OLK8DtmmB/AFndM9biU+RyxinuwEvn6j/fJLSHO/cBnmVUlL
 5aRggIXOL9zgw1i5dzA7NpFOS3b5CUbF2VkPaIKp5cB6nnziTYvBkvKcgVd1FIg37oyB
 UZ0w==
X-Gm-Message-State: APjAAAVisDWo8/+/k3q33C0zr1+ql2scsXyQQLqStK8wGOeCb4dEtDX2
 YiZbIbTDR5o/2b/aukyQkwCP1jDM
X-Google-Smtp-Source: APXvYqzauYLvwrv8F06ButzqGdlWyMSGepfsf9zUVpBXGIkKtMQB7FpjJD9e9SCGJwykZ6ekhGY/9A==
X-Received: by 2002:a5d:4b8f:: with SMTP id b15mr8124747wrt.100.1581420154691; 
 Tue, 11 Feb 2020 03:22:34 -0800 (PST)
Received: from localhost ([88.98.246.218])
 by smtp.gmail.com with ESMTPSA id l17sm4667971wro.77.2020.02.11.03.22.34
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Tue, 11 Feb 2020 03:22:34 -0800 (PST)
From: luca.boccassi@gmail.com
To: Aaron Conole <aconole@redhat.com>
Cc: David Marchand <david.marchand@redhat.com>, dpdk stable <stable@dpdk.org>
Date: Tue, 11 Feb 2020 11:19:13 +0000
Message-Id: <20200211112216.3929-7-luca.boccassi@gmail.com>
X-Mailer: git-send-email 2.20.1
In-Reply-To: <20200211112216.3929-1-luca.boccassi@gmail.com>
References: <20200211112216.3929-1-luca.boccassi@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [dpdk-stable] patch 'service: don't walk out of bounds when
	checking services' has been queued to stable release 19.11.1
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>

Hi,

FYI, your patch has been queued to stable release 19.11.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 02/13/20. 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. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Thanks.

Luca Boccassi

---
>From 09ae599c8c6bcb8dcee0c989dda75bac8ad5ee26 Mon Sep 17 00:00:00 2001
From: Aaron Conole <aconole@redhat.com>
Date: Tue, 3 Dec 2019 16:15:44 -0500
Subject: [PATCH] service: don't walk out of bounds when checking services

[ upstream commit 2e088e6f94b773233c06440763c1be43d0d705b3 ]

The service_valid call is used without properly bounds checking the
input parameter.  Almost all instances of the service_valid call are
inside a for() loop that prevents excessive walks, but some of the
public APIs don't bounds check and will pass invalid arguments.

Prevent this by using SERVICE_GET_OR_ERR_RET where it makes sense,
and adding a bounds check to one service_valid() use.

Fixes: 8d39d3e237c2 ("service: fix race in service on app lcore function")
Fixes: e9139a32f6e8 ("service: add function to run on app lcore")
Fixes: e30dd31847d2 ("service: add mechanism for quiescing")

Signed-off-by: Aaron Conole <aconole@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/rte_service.c | 32 ++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 79235c03f8..7e537b8cd2 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -137,6 +137,12 @@ service_valid(uint32_t id)
 	return !!(rte_services[id].internal_flags & SERVICE_F_REGISTERED);
 }
 
+static struct rte_service_spec_impl *
+service_get(uint32_t id)
+{
+	return &rte_services[id];
+}
+
 /* validate ID and retrieve service pointer, or return error value */
 #define SERVICE_VALID_GET_OR_ERR_RET(id, service, retval) do {          \
 	if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id))            \
@@ -344,12 +350,14 @@ rte_service_runner_do_callback(struct rte_service_spec_impl *s,
 }
 
 
-static inline int32_t
-service_run(uint32_t i, struct core_state *cs, uint64_t service_mask)
+/* Expects the service 's' is valid. */
+static int32_t
+service_run(uint32_t i, struct core_state *cs, uint64_t service_mask,
+	    struct rte_service_spec_impl *s)
 {
-	if (!service_valid(i))
+	if (!s)
 		return -EINVAL;
-	struct rte_service_spec_impl *s = &rte_services[i];
+
 	if (s->comp_runstate != RUNSTATE_RUNNING ||
 			s->app_runstate != RUNSTATE_RUNNING ||
 			!(service_mask & (UINT64_C(1) << i))) {
@@ -383,7 +391,7 @@ rte_service_may_be_active(uint32_t id)
 	int32_t lcore_count = rte_service_lcore_list(ids, RTE_MAX_LCORE);
 	int i;
 
-	if (!service_valid(id))
+	if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id))
 		return -EINVAL;
 
 	for (i = 0; i < lcore_count; i++) {
@@ -397,12 +405,10 @@ rte_service_may_be_active(uint32_t id)
 int32_t
 rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe)
 {
-	/* run service on calling core, using all-ones as the service mask */
-	if (!service_valid(id))
-		return -EINVAL;
-
 	struct core_state *cs = &lcore_states[rte_lcore_id()];
-	struct rte_service_spec_impl *s = &rte_services[id];
+	struct rte_service_spec_impl *s;
+
+	SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
 
 	/* Atomically add this core to the mapped cores first, then examine if
 	 * we can run the service. This avoids a race condition between
@@ -418,7 +424,7 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe)
 		return -EBUSY;
 	}
 
-	int ret = service_run(id, cs, UINT64_MAX);
+	int ret = service_run(id, cs, UINT64_MAX, s);
 
 	if (serialize_mt_unsafe)
 		rte_atomic32_dec(&s->num_mapped_cores);
@@ -438,8 +444,10 @@ rte_service_runner_func(void *arg)
 		const uint64_t service_mask = cs->service_mask;
 
 		for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) {
+			if (!service_valid(i))
+				continue;
 			/* return value ignored as no change to code flow */
-			service_run(i, cs, service_mask);
+			service_run(i, cs, service_mask, service_get(i));
 		}
 
 		cs->loops++;
-- 
2.20.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2020-02-11 11:17:39.196691465 +0000
+++ 0007-service-don-t-walk-out-of-bounds-when-checking-servi.patch	2020-02-11 11:17:38.307999627 +0000
@@ -1,8 +1,10 @@
-From 2e088e6f94b773233c06440763c1be43d0d705b3 Mon Sep 17 00:00:00 2001
+From 09ae599c8c6bcb8dcee0c989dda75bac8ad5ee26 Mon Sep 17 00:00:00 2001
 From: Aaron Conole <aconole@redhat.com>
 Date: Tue, 3 Dec 2019 16:15:44 -0500
 Subject: [PATCH] service: don't walk out of bounds when checking services
 
+[ upstream commit 2e088e6f94b773233c06440763c1be43d0d705b3 ]
+
 The service_valid call is used without properly bounds checking the
 input parameter.  Almost all instances of the service_valid call are
 inside a for() loop that prevents excessive walks, but some of the
@@ -14,7 +16,6 @@
 Fixes: 8d39d3e237c2 ("service: fix race in service on app lcore function")
 Fixes: e9139a32f6e8 ("service: add function to run on app lcore")
 Fixes: e30dd31847d2 ("service: add mechanism for quiescing")
-Cc: stable@dpdk.org
 
 Signed-off-by: Aaron Conole <aconole@redhat.com>
 Reviewed-by: David Marchand <david.marchand@redhat.com>