Get Outlook for Mac From: David Marchand Date: Tuesday, 8 July 2025 at 6:01 PM To: dev@dpdk.org Cc: stable@dpdk.org , Jerin Jacob , Kiran Kumar Kokkilagadda , Nithin Kumar Dabilpuram , Zhirun Yan , Pavan Nikhilesh Bhagavatula Subject: [EXTERNAL] [PATCH v3 14/18] graph: fix unaligned access in stats UBSan reports: .. /lib/graph/graph_stats. c: 208: 13: runtime error: member access within misaligned address 0x000054742c50 for type 'struct rte_graph_cluster_stats', which requires 64 byte alignment .. /lib/graph/graph_stats. c: 257: 12: runtime error:  ZjQcmQRYFpfptBannerStart Prioritize security for external emails: Confirm sender and content safety before clicking links or opening attachments Report Suspicious ZjQcmQRYFpfptBannerEnd UBSan reports: ../lib/graph/graph_stats.c:208:13: runtime error: member access within misaligned address 0x000054742c50 for type 'struct rte_graph_cluster_stats', which requires 64 byte alignment ../lib/graph/graph_stats.c:257:12: runtime error: member access within misaligned address 0x00002219fd30 for type 'struct rte_graph_cluster_stats', which requires 64 byte alignment The current code goes into various complex (non aligned) reallocations / memset / memcpy. Simplify this by computing how many nodes are present in the cluster of graphes. Then directly call rte_malloc for the whole stats object. As a bonus, this change also fixes leaks: - if any error occurred before call to rte_malloc, since the xstats objects stored in the glibc allocated stats object were not freed, - if an allocation failure occurs, with constructs using ptr = realloc(ptr, sz), since the original ptr is lost, Fixes: af1ae8b6a32c ("graph: implement stats") Cc: stable@dpdk.org Signed-off-by: David Marchand Acked-by: Kiran Kumar Kokkilagadda > lib/graph/graph_stats.c | 102 +++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 44 deletions(-) diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c index 583ad8dbd5..e9b183bf13 100644 --- a/lib/graph/graph_stats.c +++ b/lib/graph/graph_stats.c @@ -37,7 +37,6 @@ struct __rte_cache_aligned rte_graph_cluster_stats { int socket_id; bool dispatch; void *cookie; - size_t sz; struct cluster_node clusters[]; }; @@ -178,15 +177,55 @@ graph_cluster_stats_cb_dispatch(bool is_first, bool is_last, void *cookie, return graph_cluster_stats_cb(true, is_first, is_last, cookie, stat); }; +static uint32_t +cluster_count_nodes(const struct cluster *cluster) +{ + rte_node_t *nodes = NULL; + uint32_t max_nodes = 0; + + for (unsigned int i = 0; i < cluster->nb_graphs; i++) { + struct graph_node *graph_node; + + STAILQ_FOREACH(graph_node, &cluster->graphs[i]->node_list, next) { + unsigned int n; + + for (n = 0; n < max_nodes; n++) { + if (nodes[n] != graph_node->node->id) + continue; + break; + } + if (n == max_nodes) { + rte_node_t *new_nodes; + + max_nodes++; + new_nodes = realloc(nodes, max_nodes * sizeof(nodes[0])); + if (new_nodes == NULL) { + free(nodes); + return 0; + } + nodes = new_nodes; + nodes[n] = graph_node->node->id; + } + } + } + free(nodes); + + return max_nodes; +} + static struct rte_graph_cluster_stats * stats_mem_init(struct cluster *cluster, const struct rte_graph_cluster_stats_param *prm) { - size_t sz = sizeof(struct rte_graph_cluster_stats); struct rte_graph_cluster_stats *stats; rte_graph_cluster_stats_cb_t fn; int socket_id = prm->socket_id; uint32_t cluster_node_size; + uint32_t max_nodes; + + max_nodes = cluster_count_nodes(cluster); + if (max_nodes == 0) + return NULL; /* Fix up callback */ fn = prm->fn; @@ -203,25 +242,23 @@ stats_mem_init(struct cluster *cluster, cluster_node_size += cluster->nb_graphs * sizeof(struct rte_node *); cluster_node_size = RTE_ALIGN(cluster_node_size, RTE_CACHE_LINE_SIZE); - stats = realloc(NULL, sz); + stats = rte_zmalloc_socket(NULL, sizeof(struct rte_graph_cluster_stats) + + max_nodes * cluster_node_size, 0, socket_id); if (stats) { - memset(stats, 0, sz); stats->fn = fn; stats->cluster_node_size = cluster_node_size; stats->max_nodes = 0; stats->socket_id = socket_id; stats->cookie = prm->cookie; - stats->sz = sz; } return stats; } static int -stats_mem_populate(struct rte_graph_cluster_stats **stats_in, +stats_mem_populate(struct rte_graph_cluster_stats *stats, struct rte_graph *graph, struct graph_node *graph_node) { - struct rte_graph_cluster_stats *stats = *stats_in; rte_node_t id = graph_node->node->id; struct cluster_node *cluster; struct rte_node *node; @@ -247,21 +284,12 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in, cluster = RTE_PTR_ADD(cluster, stats->cluster_node_size); } - /* Hey, it is a new node, allocate space for it in the reel */ - stats = realloc(stats, stats->sz + stats->cluster_node_size); - if (stats == NULL) - SET_ERR_JMP(ENOMEM, err, "Realloc failed"); - *stats_in = NULL; - - /* Clear the new struct cluster_node area */ - cluster = RTE_PTR_ADD(stats, stats->sz), - memset(cluster, 0, stats->cluster_node_size); memcpy(cluster->stat.name, graph_node->node->name, RTE_NODE_NAMESIZE); cluster->stat.id = graph_node->node->id; cluster->stat.hz = rte_get_timer_hz(); node = graph_node_id_to_ptr(graph, id); if (node == NULL) - SET_ERR_JMP(ENOENT, free, "Failed to find node %s in graph %s", + SET_ERR_JMP(ENOENT, err, "Failed to find node %s in graph %s", graph_node->node->name, graph->name); cluster->nodes[cluster->nb_nodes++] = node; if (graph_node->node->xstats) { @@ -270,7 +298,7 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in, sizeof(uint64_t) * graph_node->node->xstats->nb_xstats, RTE_CACHE_LINE_SIZE, stats->socket_id); if (cluster->stat.xstat_count == NULL) - SET_ERR_JMP(ENOMEM, free, "Failed to allocate memory node %s graph %s", + SET_ERR_JMP(ENOMEM, err, "Failed to allocate memory node %s graph %s", graph_node->node->name, graph->name); cluster->stat.xstat_desc = rte_zmalloc_socket(NULL, @@ -278,7 +306,7 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in, RTE_CACHE_LINE_SIZE, stats->socket_id); if (cluster->stat.xstat_desc == NULL) { rte_free(cluster->stat.xstat_count); - SET_ERR_JMP(ENOMEM, free, "Failed to allocate memory node %s graph %s", + SET_ERR_JMP(ENOMEM, err, "Failed to allocate memory node %s graph %s", graph_node->node->name, graph->name); } @@ -288,30 +316,20 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in, RTE_NODE_XSTAT_DESC_SIZE) < 0) { rte_free(cluster->stat.xstat_count); rte_free(cluster->stat.xstat_desc); - SET_ERR_JMP(E2BIG, free, + SET_ERR_JMP(E2BIG, err, "Error description overflow node %s graph %s", graph_node->node->name, graph->name); } } } - stats->sz += stats->cluster_node_size; stats->max_nodes++; - *stats_in = stats; return 0; -free: - free(stats); err: return -rte_errno; } -static void -stats_mem_fini(struct rte_graph_cluster_stats *stats) -{ - free(stats); -} - static void cluster_init(struct cluster *cluster) { @@ -381,10 +399,7 @@ struct rte_graph_cluster_stats * rte_graph_cluster_stats_create(const struct rte_graph_cluster_stats_param *prm) { struct rte_graph_cluster_stats *stats, *rc = NULL; - struct graph_node *graph_node; struct cluster cluster; - struct graph *graph; - const char *pattern; rte_graph_t i; /* Sanity checks */ @@ -402,37 +417,36 @@ rte_graph_cluster_stats_create(const struct rte_graph_cluster_stats_param *prm) graph_spinlock_lock(); /* Expand graph pattern and add the graph to the cluster */ for (i = 0; i < prm->nb_graph_patterns; i++) { - pattern = prm->graph_patterns[i]; - if (expand_pattern_to_cluster(&cluster, pattern)) + if (expand_pattern_to_cluster(&cluster, prm->graph_patterns[i])) goto bad_pattern; } /* Alloc the stats memory */ stats = stats_mem_init(&cluster, prm); if (stats == NULL) - SET_ERR_JMP(ENOMEM, bad_pattern, "Failed alloc stats memory"); + SET_ERR_JMP(ENOMEM, bad_pattern, "Failed rte_malloc for stats memory"); /* Iterate over M(Graph) x N (Nodes in graph) */ for (i = 0; i < cluster.nb_graphs; i++) { + struct graph_node *graph_node; + struct graph *graph; + graph = cluster.graphs[i]; STAILQ_FOREACH(graph_node, &graph->node_list, next) { struct rte_graph *graph_fp = graph->graph; - if (stats_mem_populate(&stats, graph_fp, graph_node)) + if (stats_mem_populate(stats, graph_fp, graph_node)) goto realloc_fail; } if (graph->graph->model == RTE_GRAPH_MODEL_MCORE_DISPATCH) stats->dispatch = true; } - /* Finally copy to hugepage memory to avoid pressure on rte_realloc */ - rc = rte_malloc_socket(NULL, stats->sz, 0, stats->socket_id); - if (rc) - rte_memcpy(rc, stats, stats->sz); - else - SET_ERR_JMP(ENOMEM, realloc_fail, "rte_malloc failed"); + rc = stats; + stats = NULL; realloc_fail: - stats_mem_fini(stats); + if (stats != NULL) + rte_graph_cluster_stats_destroy(stats); bad_pattern: graph_spinlock_unlock(); cluster_fini(&cluster); -- 2.50.0