From patchwork Mon Mar 25 15:53:55 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Robin Jarry X-Patchwork-Id: 138768 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 31C6D43D47; Mon, 25 Mar 2024 16:54:21 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2391C40A79; Mon, 25 Mar 2024 16:54:21 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 73FD640A77 for ; Mon, 25 Mar 2024 16:54:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711382059; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=g+6Jl8WxduhGWl/wkZtWM9Ql8JciOSk93HaJOUGzNvE=; b=ME6byyjPqsP4kz4hXuxNszPpTAawQ7hFL8dWFjNLEVJl378oCaOOoT9ZR9N2lXxvFVbe3f Rid8N4EDwmH+gFBWlkw+5FB6RSC++f4sce2LIzUoX6So2nz4npvBjPRKicfKyBqwPSZt2L l6jdkopw9oeUK8FveLrNfevq+kmdBEc= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-323-N5LFCUvCO2K8dIyKiuV1Ug-1; Mon, 25 Mar 2024 11:54:14 -0400 X-MC-Unique: N5LFCUvCO2K8dIyKiuV1Ug-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 47008185A786; Mon, 25 Mar 2024 15:54:14 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.208.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id E5F6510E47; Mon, 25 Mar 2024 15:54:12 +0000 (UTC) From: Robin Jarry To: dev@dpdk.org, Jerin Jacob , Kiran Kumar K , Nithin Dabilpuram , Zhirun Yan Subject: [PATCH] graph: avoid id collisions Date: Mon, 25 Mar 2024 16:53:55 +0100 Message-ID: <20240325155354.770676-2-rjarry@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org The graph id is determined based on a global variable that is incremented every time a graph is created, and decremented every time a graph is destroyed. This only works if graphs are destroyed in the reverse order in which they have been created. The following code produces duplicate graph IDs which can lead to use-after-free bugs and other undefined behaviours: a = rte_graph_create(...); // id=0 graph_id=1 b = rte_graph_create(...); // id=1 graph_id=2 rte_graph_destroy(a); // graph_id=1 c = rte_graph_create(...); // id=1 graph_id=2 (duplicate with b) rte_graph_destroy(c); // frees memory still used by b Remove the global counter. Make sure that the graph list is always ordered by increasing graph ids. When creating a new graph, pick a free id which is not allocated. Signed-off-by: Robin Jarry --- lib/graph/graph.c | 86 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 69 insertions(+), 17 deletions(-) diff --git a/lib/graph/graph.c b/lib/graph/graph.c index 147bc6c685c5..50616580d06b 100644 --- a/lib/graph/graph.c +++ b/lib/graph/graph.c @@ -19,11 +19,54 @@ static struct graph_head graph_list = STAILQ_HEAD_INITIALIZER(graph_list); static rte_spinlock_t graph_lock = RTE_SPINLOCK_INITIALIZER; -static rte_graph_t graph_id; - -#define GRAPH_ID_CHECK(id) ID_CHECK(id, graph_id) /* Private functions */ +static struct graph * +graph_from_id(rte_graph_t id) +{ + struct graph *graph; + STAILQ_FOREACH(graph, &graph_list, next) { + if (graph->id == id) + return graph; + } + rte_errno = EINVAL; + return NULL; +} + +static rte_graph_t +graph_next_free_id(void) +{ + struct graph *graph; + rte_graph_t id = 0; + + STAILQ_FOREACH(graph, &graph_list, next) { + if (id < graph->id) + break; + id = graph->id + 1; + } + + return id; +} + +static void +graph_insert_ordered(struct graph *graph) +{ + struct graph *after, *g; + + after = NULL; + STAILQ_FOREACH(g, &graph_list, next) { + if (g->id < graph->id) { + after = g; + break; + } + } + if (after == NULL) { + STAILQ_INSERT_TAIL(&graph_list, graph, next); + } else { + STAILQ_INSERT_AFTER(&graph_list, after, graph, next); + } +} + struct graph_head * graph_list_head_get(void) { @@ -279,7 +322,8 @@ rte_graph_model_mcore_dispatch_core_bind(rte_graph_t id, int lcore) { struct graph *graph; - GRAPH_ID_CHECK(id); + if (graph_from_id(id) == NULL) + goto fail; if (!rte_lcore_is_enabled(lcore)) SET_ERR_JMP(ENOLINK, fail, "lcore %d not enabled", lcore); @@ -309,7 +353,8 @@ rte_graph_model_mcore_dispatch_core_unbind(rte_graph_t id) { struct graph *graph; - GRAPH_ID_CHECK(id); + if (graph_from_id(id) == NULL) + goto fail; STAILQ_FOREACH(graph, &graph_list, next) if (graph->id == id) break; @@ -406,7 +451,7 @@ rte_graph_create(const char *name, struct rte_graph_param *prm) graph->socket = prm->socket_id; graph->src_node_count = src_node_count; graph->node_count = graph_nodes_count(graph); - graph->id = graph_id; + graph->id = graph_next_free_id(); graph->parent_id = RTE_GRAPH_ID_INVALID; graph->lcore_id = RTE_MAX_LCORE; graph->num_pkt_to_capture = prm->num_pkt_to_capture; @@ -422,8 +467,7 @@ rte_graph_create(const char *name, struct rte_graph_param *prm) goto graph_mem_destroy; /* All good, Lets add the graph to the list */ - graph_id++; - STAILQ_INSERT_TAIL(&graph_list, graph, next); + graph_insert_ordered(graph); graph_spinlock_unlock(); return graph->id; @@ -467,7 +511,6 @@ rte_graph_destroy(rte_graph_t id) graph_cleanup(graph); STAILQ_REMOVE(&graph_list, graph, graph, next); free(graph); - graph_id--; goto done; } graph = tmp; @@ -520,7 +563,7 @@ graph_clone(struct graph *parent_graph, const char *name, struct rte_graph_param graph->parent_id = parent_graph->id; graph->lcore_id = parent_graph->lcore_id; graph->socket = parent_graph->socket; - graph->id = graph_id; + graph->id = graph_next_free_id(); /* Allocate the Graph fast path memory and populate the data */ if (graph_fp_mem_create(graph)) @@ -539,8 +582,7 @@ graph_clone(struct graph *parent_graph, const char *name, struct rte_graph_param goto graph_mem_destroy; /* All good, Lets add the graph to the list */ - graph_id++; - STAILQ_INSERT_TAIL(&graph_list, graph, next); + graph_insert_ordered(graph); graph_spinlock_unlock(); return graph->id; @@ -561,7 +603,8 @@ rte_graph_clone(rte_graph_t id, const char *name, struct rte_graph_param *prm) { struct graph *graph; - GRAPH_ID_CHECK(id); + if (graph_from_id(id) == NULL) + goto fail; STAILQ_FOREACH(graph, &graph_list, next) if (graph->id == id) return graph_clone(graph, name, prm); @@ -587,7 +630,8 @@ rte_graph_id_to_name(rte_graph_t id) { struct graph *graph; - GRAPH_ID_CHECK(id); + if (graph_from_id(id) == NULL) + goto fail; STAILQ_FOREACH(graph, &graph_list, next) if (graph->id == id) return graph->name; @@ -604,7 +648,8 @@ rte_graph_node_get(rte_graph_t gid, uint32_t nid) rte_graph_off_t off; rte_node_t count; - GRAPH_ID_CHECK(gid); + if (graph_from_id(gid) == NULL) + goto fail; STAILQ_FOREACH(graph, &graph_list, next) if (graph->id == gid) { rte_graph_foreach_node(count, off, graph->graph, @@ -747,7 +792,8 @@ graph_scan_dump(FILE *f, rte_graph_t id, bool all) struct graph *graph; RTE_VERIFY(f); - GRAPH_ID_CHECK(id); + if (graph_from_id(id) == NULL) + goto fail; STAILQ_FOREACH(graph, &graph_list, next) { if (all == true) { @@ -776,7 +822,13 @@ rte_graph_list_dump(FILE *f) rte_graph_t rte_graph_max_count(void) { - return graph_id; + struct graph *graph; + rte_graph_t count = 0; + + STAILQ_FOREACH(graph, &graph_list, next) + count++; + + return count; } RTE_LOG_REGISTER_DEFAULT(rte_graph_logtype, INFO);