From: Aaron Conole <aconole@redhat.com> To: dev@dpdk.org Cc: Harry van Haaren <harry.van.haaren@intel.com>, Bruce Richardson <bruce.richardson@intel.com>, Pavan Nikhilesh <pbhagavatula@marvell.com>, Gage Eads <gage.eads@intel.com>, Thomas Monjalon <thomas@monjalon.net>, David Marchand <dmarchan@redhat.com> Subject: [dpdk-dev] [PATCH] service: don't walk out of bounds when checking services Date: Tue, 26 Nov 2019 09:56:06 -0500 Message-ID: <20191126145606.13626-1-aconole@redhat.com> (raw) 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> --- lib/librte_eal/common/rte_service.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 79235c03f8..73de7bbade 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -345,11 +345,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) +service_run(uint32_t i, struct core_state *cs, uint64_t service_mask, + struct rte_service_spec_impl *s) { - if (!service_valid(i)) - return -EINVAL; - struct rte_service_spec_impl *s = &rte_services[i]; + if (!s) + SERVICE_VALID_GET_OR_ERR_RET(i, s, -EINVAL); + if (s->comp_runstate != RUNSTATE_RUNNING || s->app_runstate != RUNSTATE_RUNNING || !(service_mask & (UINT64_C(1) << i))) { @@ -383,7 +384,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 +398,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 +417,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); @@ -439,7 +438,7 @@ rte_service_runner_func(void *arg) for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { /* return value ignored as no change to code flow */ - service_run(i, cs, service_mask); + service_run(i, cs, service_mask, NULL); } cs->loops++; -- 2.21.0
next reply index Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-26 14:56 Aaron Conole [this message] 2019-12-02 16:16 ` Eads, Gage 2019-12-02 16:19 ` David Marchand 2019-12-03 15:10 ` Aaron Conole 2019-12-03 21:15 ` [dpdk-dev] [PATCH v2] " Aaron Conole 2019-12-04 8:33 ` David Marchand 2019-12-04 8:34 ` David Marchand 2019-12-20 14:43 ` David Marchand 2020-02-07 12:04 ` Kevin Traynor 2020-02-07 14:27 ` Aaron Conole 2020-02-14 16:38 ` Kevin Traynor
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191126145606.13626-1-aconole@redhat.com \ --to=aconole@redhat.com \ --cc=bruce.richardson@intel.com \ --cc=dev@dpdk.org \ --cc=dmarchan@redhat.com \ --cc=gage.eads@intel.com \ --cc=harry.van.haaren@intel.com \ --cc=pbhagavatula@marvell.com \ --cc=thomas@monjalon.net \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
DPDK-dev Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \ dev@dpdk.org public-inbox-index dpdk-dev Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.dpdk.dev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git