From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2346DA051C for ; 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 ; Tue, 11 Feb 2020 12:22:34 +0100 (CET) Received: by mail-wr1-f41.google.com with SMTP id g3so10775373wrs.12 for ; 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 Cc: David Marchand , dpdk stable 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" 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 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 Reviewed-by: David Marchand --- 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 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 Reviewed-by: David Marchand