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 4948EA04DD; Tue, 26 Nov 2019 15:56:17 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 238B85596; Tue, 26 Nov 2019 15:56:17 +0100 (CET) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by dpdk.org (Postfix) with ESMTP id AEDA93195 for ; Tue, 26 Nov 2019 15:56:15 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574780175; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=dwNtcRdK7O+a7o7B71WtMI/Ys75dNiiqwUj2Zu2Wcow=; b=Bk4J/dUlYnJfiGVG1UPF9857f/o4RHrg17VKgwMlnoAjI6gbA0hNI02vmStmoqlPkZPqZ3 4Y4v3XyNjekrgYQlTxgUDSOVdrOuLBXhi11+prGr8WYDf/Yi7gJEOUEiJ8GOtZiEPGPpUZ Lvn7w93XkUoYmjyvdSFt4RxFqVaS9aw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-324-0vLo7lKeO12XP22BYzJ7kw-1; Tue, 26 Nov 2019 09:56:11 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id D7727802FD5; Tue, 26 Nov 2019 14:56:09 +0000 (UTC) Received: from dhcp-25.97.bos.redhat.com (unknown [10.18.25.127]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2057C60137; Tue, 26 Nov 2019 14:56:07 +0000 (UTC) From: Aaron Conole To: dev@dpdk.org Cc: Harry van Haaren , Bruce Richardson , Pavan Nikhilesh , Gage Eads , Thomas Monjalon , David Marchand Date: Tue, 26 Nov 2019 09:56:06 -0500 Message-Id: <20191126145606.13626-1-aconole@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-MC-Unique: 0vLo7lKeO12XP22BYzJ7kw-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Subject: [dpdk-dev] [PATCH] service: don't walk out of bounds when checking services X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 --- 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/rt= e_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_spe= c_impl *s, =20 =20 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, +=09 struct rte_service_spec_impl *s) { -=09if (!service_valid(i)) -=09=09return -EINVAL; -=09struct rte_service_spec_impl *s =3D &rte_services[i]; +=09if (!s) +=09=09SERVICE_VALID_GET_OR_ERR_RET(i, s, -EINVAL); + =09if (s->comp_runstate !=3D RUNSTATE_RUNNING || =09=09=09s->app_runstate !=3D RUNSTATE_RUNNING || =09=09=09!(service_mask & (UINT64_C(1) << i))) { @@ -383,7 +384,7 @@ rte_service_may_be_active(uint32_t id) =09int32_t lcore_count =3D rte_service_lcore_list(ids, RTE_MAX_LCORE); =09int i; =20 -=09if (!service_valid(id)) +=09if (id >=3D RTE_SERVICE_NUM_MAX || !service_valid(id)) =09=09return -EINVAL; =20 =09for (i =3D 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_unsaf= e) { -=09/* run service on calling core, using all-ones as the service mask */ -=09if (!service_valid(id)) -=09=09return -EINVAL; - =09struct core_state *cs =3D &lcore_states[rte_lcore_id()]; -=09struct rte_service_spec_impl *s =3D &rte_services[id]; +=09struct rte_service_spec_impl *s; + +=09SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); =20 =09/* Atomically add this core to the mapped cores first, then examine if =09 * 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) =09=09return -EBUSY; =09} =20 -=09int ret =3D service_run(id, cs, UINT64_MAX); +=09int ret =3D service_run(id, cs, UINT64_MAX, s); =20 =09if (serialize_mt_unsafe) =09=09rte_atomic32_dec(&s->num_mapped_cores); @@ -439,7 +438,7 @@ rte_service_runner_func(void *arg) =20 =09=09for (i =3D 0; i < RTE_SERVICE_NUM_MAX; i++) { =09=09=09/* return value ignored as no change to code flow */ -=09=09=09service_run(i, cs, service_mask); +=09=09=09service_run(i, cs, service_mask, NULL); =09=09} =20 =09=09cs->loops++; --=20 2.21.0