From bfa8d6b1b8aaef9e793c1e612f88377f55646ad7 Mon Sep 17 00:00:00 2001 From: Binbin Date: Mon, 17 Nov 2025 17:50:37 +0800 Subject: [PATCH 1/3] Check for duplicate nodeids when loading nodes.conf For corrupted (human-made) or program-error-recovered nodes.conf files, check for duplicate nodeids when loading nodes.conf. If a duplicate is found, panic is triggered to prevent nodes from starting up unexpectedly. The node ID is used to identify every node across the whole cluster, we do not expect to find duplicate nodeids in nodes.conf. Signed-off-by: Binbin --- src/cluster_legacy.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c index b7df963b4a..6f79ba12fc 100644 --- a/src/cluster_legacy.c +++ b/src/cluster_legacy.c @@ -684,6 +684,10 @@ int clusterLoadConfig(char *filename) { if (!n) { n = createClusterNode(argv[0], 0); clusterAddNode(n); + } else { + serverLog(LL_WARNING, "Duplicate nodeid detected: %s", argv[0]); + sdsfreesplitres(argv, argc); + goto fmterr; } /* Format for the node address and auxiliary argument information: * ip:port[@cport][,hostname][,aux=val]*] */ From 96159e07a3fc331e70a91645b4312e77cd94bdf6 Mon Sep 17 00:00:00 2001 From: Binbin Date: Mon, 17 Nov 2025 20:05:48 +0800 Subject: [PATCH 2/3] more code Signed-off-by: Binbin --- src/cluster_legacy.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c index 6f79ba12fc..f118f68873 100644 --- a/src/cluster_legacy.c +++ b/src/cluster_legacy.c @@ -607,6 +607,7 @@ int clusterLoadConfig(char *filename) { struct stat sb; char *line; int maxline, j; + dict *tmp_cluster_nodes; if (fp == NULL) { if (errno == ENOENT) { @@ -637,6 +638,7 @@ int clusterLoadConfig(char *filename) { * To simplify we allocate 1024+CLUSTER_SLOTS*128 bytes per line. */ maxline = 1024 + CLUSTER_SLOTS * 128; line = zmalloc(maxline); + tmp_cluster_nodes = dictCreate(&clusterNodesDictType); while (fgets(line, maxline, fp) != NULL) { int argc, aux_argc; sds *argv, *aux_argv; @@ -684,10 +686,19 @@ int clusterLoadConfig(char *filename) { if (!n) { n = createClusterNode(argv[0], 0); clusterAddNode(n); + dictAdd(tmp_cluster_nodes, sdsnewlen(argv[0], sdslen(argv[0])), NULL); } else { - serverLog(LL_WARNING, "Duplicate nodeid detected: %s", argv[0]); - sdsfreesplitres(argv, argc); - goto fmterr; + /* Check if the node (nodeid) has already been loaded. The nodeid is used to + * identify every node across the entire cluster, we do not expect to find + * duplicate nodeids in nodes.conf. */ + sds nodename = sdsnewlen(argv[0], sdslen(argv[0])); + dictEntry *de = dictFind(tmp_cluster_nodes, nodename); + sdsfree(nodename); + if (de != NULL) { + serverLog(LL_WARNING, "Duplicate nodeid detected: %s", argv[0]); + sdsfreesplitres(argv, argc); + goto fmterr; + } } /* Format for the node address and auxiliary argument information: * ip:port[@cport][,hostname][,aux=val]*] */ @@ -944,6 +955,7 @@ int clusterLoadConfig(char *filename) { zfree(line); fclose(fp); + if (tmp_cluster_nodes) dictRelease(tmp_cluster_nodes); serverLog(LL_NOTICE, "Node configuration loaded, I'm %.40s", myself->name); From d2db0585a02aa266b46f81d9f0b3b5db558ec015 Mon Sep 17 00:00:00 2001 From: Binbin Date: Tue, 2 Dec 2025 12:17:31 +0800 Subject: [PATCH 3/3] code review Signed-off-by: Binbin --- src/cluster_legacy.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c index 1b6770e9f5..9bc5b24adb 100644 --- a/src/cluster_legacy.c +++ b/src/cluster_legacy.c @@ -694,9 +694,7 @@ int clusterLoadConfig(char *filename) { /* Check if the node (nodeid) has already been loaded. The nodeid is used to * identify every node across the entire cluster, we do not expect to find * duplicate nodeids in nodes.conf. */ - sds nodename = sdsnewlen(argv[0], sdslen(argv[0])); - dictEntry *de = dictFind(tmp_cluster_nodes, nodename); - sdsfree(nodename); + dictEntry *de = dictFind(tmp_cluster_nodes, argv[0]); if (de != NULL) { serverLog(LL_WARNING, "Duplicate nodeid detected: %s", argv[0]); sdsfreesplitres(argv, argc); @@ -958,7 +956,8 @@ int clusterLoadConfig(char *filename) { zfree(line); fclose(fp); - if (tmp_cluster_nodes) dictRelease(tmp_cluster_nodes); + serverAssert(tmp_cluster_nodes != NULL); + dictRelease(tmp_cluster_nodes); serverLog(LL_NOTICE, "Node configuration loaded, I'm %.40s", myself->name);