[dpdk-stable] patch 'service: don't walk out of bounds when checking services' has been queued to LTS release 18.11.7

[Kevin Traynor]

Hi,

FYI, your patch has been queued to LTS release 18.11.7

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.

Queued patches are on a temporary branch at:
https://github.com/kevintraynor/dpdk-stable-queue

This queued commit can be viewed at:
https://github.com/kevintraynor/dpdk-stable-queue/commit/dd4c770c5e356915e9ee134f0baba286ce3784cc

Thanks.

Kevin.

---
>From dd4c770c5e356915e9ee134f0baba286ce3784cc Mon Sep 17 00:00:00 2001
From: Aaron Conole <aconole at 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 at redhat.com>
Reviewed-by: David Marchand <david.marchand at 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 97449460dc..53dd6a7bbf 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -137,4 +137,10 @@ service_valid(uint32_t id)
 }
 
+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 {          \
@@ -344,10 +350,12 @@ 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 ||
@@ -383,5 +391,5 @@ rte_service_may_be_active(uint32_t id)
 	int i;
 
-	if (!service_valid(id))
+	if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id))
 		return -EINVAL;
 
@@ -397,10 +405,8 @@ 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
@@ -418,5 +424,5 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe)
 	}
 
-	int ret = service_run(id, cs, UINT64_MAX);
+	int ret = service_run(id, cs, UINT64_MAX, s);
 
 	if (serialize_mt_unsafe)
@@ -438,6 +444,8 @@ rte_service_runner_func(void *arg)
 
 		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));
 		}
 
-- 
2.21.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2020-02-07 15:08:19.393876737 +0000
+++ 0036-service-don-t-walk-out-of-bounds-when-checking-servi.patch	2020-02-07 15:08:17.572062179 +0000
@@ -1 +1 @@
-From 2e088e6f94b773233c06440763c1be43d0d705b3 Mon Sep 17 00:00:00 2001
+From dd4c770c5e356915e9ee134f0baba286ce3784cc Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit 2e088e6f94b773233c06440763c1be43d0d705b3 ]
+
@@ -17 +18,0 @@
-Cc: stable at dpdk.org
@@ -26 +27 @@
-index 79235c03f8..7e537b8cd2 100644
+index 97449460dc..53dd6a7bbf 100644
@@ -29 +30 @@
-@@ -138,4 +138,10 @@ service_valid(uint32_t id)
+@@ -137,4 +137,10 @@ service_valid(uint32_t id)
@@ -40 +41 @@
-@@ -345,10 +351,12 @@ rte_service_runner_do_callback(struct rte_service_spec_impl *s,
+@@ -344,10 +350,12 @@ rte_service_runner_do_callback(struct rte_service_spec_impl *s,
@@ -57 +58 @@
-@@ -384,5 +392,5 @@ rte_service_may_be_active(uint32_t id)
+@@ -383,5 +391,5 @@ rte_service_may_be_active(uint32_t id)
@@ -64 +65 @@
-@@ -398,10 +406,8 @@ int32_t
+@@ -397,10 +405,8 @@ int32_t
@@ -78 +79 @@
-@@ -419,5 +425,5 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe)
+@@ -418,5 +424,5 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe)
@@ -85 +86 @@
-@@ -439,6 +445,8 @@ rte_service_runner_func(void *arg)
+@@ -438,6 +444,8 @@ rte_service_runner_func(void *arg)