From dbef0cfd025b858d5690cc0a739bc2bb55fb268f Mon Sep 17 00:00:00 2001 From: ChengqiangMeng Date: Tue, 20 Apr 2021 15:25:04 +0800 Subject: [PATCH] app/spdk_top: fix SEGV on unknown address The index of g_thread_info is equals to the thread ID. When the thread ID is not continuous, but index of ABC is continuous, so some elements of g_thread_info array will be empty. fixes #1899 Signed-off-by: ChengqiangMeng Change-Id: Ib90a26dcc2d47792a098b163746906f34043453a Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7493 Tested-by: SPDK CI Jenkins Reviewed-by: Krzysztof Karas Reviewed-by: Tomasz Zawadzki Reviewed-by: Jim Harris --- app/spdk_top/spdk_top.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/app/spdk_top/spdk_top.c b/app/spdk_top/spdk_top.c index 199263282..9c2a3a801 100644 --- a/app/spdk_top/spdk_top.c +++ b/app/spdk_top/spdk_top.c @@ -557,6 +557,7 @@ get_data(void) } /* Decode json */ + memset(&g_threads_stats, 0, sizeof(g_threads_stats)); if (spdk_json_decode_object(json_resp->result, rpc_threads_stats_decoders, SPDK_COUNTOF(rpc_threads_stats_decoders), &g_threads_stats)) { rc = -EINVAL; @@ -799,6 +800,16 @@ sort_threads(const void *p1, const void *p2) const struct rpc_thread_info *thread_info2 = *(struct rpc_thread_info **)p2; uint64_t count1, count2; + /* thread IDs may not be allocated contiguously, so we need + * to account for NULL thread_info pointers */ + if (thread_info1 == NULL && thread_info2 == NULL) { + return 0; + } else if (thread_info1 == NULL) { + return 1; + } else if (thread_info2 == NULL) { + return -1; + } + switch (g_current_sort_col[THREADS_TAB]) { case 0: /* Sort by name */ return strcmp(thread_info1->name, thread_info2->name); @@ -878,10 +889,15 @@ refresh_threads_tab(uint8_t current_page) g_last_threads_count = threads_count; } - /* Thread IDs starts from '1', so we have to take this into account when copying. - * TODO: In future we can have gaps in ID list, so we will need to change the way we - * handle copying threads list below */ - memcpy(thread_info, &g_thread_info[1], sizeof(struct rpc_thread_info *) * threads_count); + /* From g_thread_info copy to thread_info without null elements. + * The index of g_thread_info equals to Thread IDs, so it starts from '1'. */ + for (i = 0, j = 1; i < g_threads_stats.threads.threads_count; i++) { + while (g_thread_info[j] == NULL) { + j++; + } + memcpy(&thread_info[i], &g_thread_info[j], sizeof(struct rpc_thread_info *)); + j++; + } if (last_page != current_page) { for (i = 0; i < threads_count; i++) {