Skip to content

Commit 19d2da2

Browse files
committed
CDRIVER-695 crash destroying node after auth err
Avoid scenarios like: 1. Connect to 2-node replica set. 2. _cluster_reconnect_replica_set enters first loop, calls ismaster on primary and finds two peers. 3. nodes_len is set to 2 and the nodes list is realloc'ed, but the second node is uninitialized. 4. _mongoc_cluster_reconnect_replica_set enters second loop. 5. Auth fails, "goto CLEANUP". 6. Now nodes_len is 2 but the second node is still uninitialized. 7. Later, _mongoc_cluster_node_destroy iterates over both nodes. 8. Destroying second, uninitialized node calls stream->close, which is a random location, segfaults.
1 parent 3f43d63 commit 19d2da2

File tree

5 files changed

+249
-8
lines changed

5 files changed

+249
-8
lines changed

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ set(test-libmongoc-sources
212212
${SOURCE_DIR}/tests/mock-server.c
213213
${SOURCE_DIR}/tests/test-bulk.c
214214
${SOURCE_DIR}/tests/test-mongoc-client-pool.c
215+
${SOURCE_DIR}/tests/test-mongoc-cluster.c
215216
${SOURCE_DIR}/tests/test-mongoc-collection.c
216217
${SOURCE_DIR}/tests/test-mongoc-cursor.c
217218
${SOURCE_DIR}/tests/test-mongoc-database.c

src/mongoc/mongoc-cluster.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2202,13 +2202,14 @@ _mongoc_cluster_reconnect_replica_set (mongoc_cluster_t *cluster,
22022202
}
22032203
}
22042204

2205+
/* reinitialize nodes with same length as peer list. */
22052206
for (liter = list, i = 0; liter; liter = liter->next, i++) {}
22062207
cluster->nodes = bson_realloc (cluster->nodes, sizeof (*cluster->nodes) * i);
2207-
cluster->nodes_len = i;
22082208

2209-
/* guard against counter errors, see CDRIVER-695 */
2210-
for (i = 0; i < cluster->nodes_len; i++) {
2211-
cluster->nodes[i].valid = false;
2209+
for (j = 0; j < i; j++) {
2210+
_mongoc_cluster_node_init (&cluster->nodes[j]);
2211+
/* guard against counter errors, see CDRIVER-695 */
2212+
cluster->nodes[j].valid = false;
22122213
}
22132214

22142215
for (liter = list, i = 0; liter; liter = liter->next) {
@@ -2242,8 +2243,6 @@ _mongoc_cluster_reconnect_replica_set (mongoc_cluster_t *cluster,
22422243
}
22432244
}
22442245

2245-
_mongoc_cluster_node_init(&cluster->nodes[i]);
2246-
22472246
cluster->nodes[i].host = host;
22482247
cluster->nodes[i].index = i;
22492248
cluster->nodes[i].stream = stream;
@@ -2285,10 +2284,9 @@ _mongoc_cluster_reconnect_replica_set (mongoc_cluster_t *cluster,
22852284

22862285
cluster->nodes[i].valid = true;
22872286
i++;
2287+
cluster->nodes_len = (uint32_t) i;
22882288
}
22892289

2290-
cluster->nodes_len = i;
2291-
22922290
_mongoc_list_foreach(list, (void(*)(void*,void*))bson_free, NULL);
22932291
_mongoc_list_destroy(list);
22942292

tests/Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ test_libmongoc_SOURCES = \
7979
tests/test-mongoc-buffer.c \
8080
tests/test-mongoc-client.c \
8181
tests/test-mongoc-client-pool.c \
82+
tests/test-mongoc-cluster.c \
8283
tests/test-mongoc-collection.c \
8384
tests/test-mongoc-cursor.c \
8485
tests/test-mongoc-database.c \

tests/test-libmongoc.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ extern void test_buffer_install (TestSuite *suite);
3030
extern void test_bulk_install (TestSuite *suite);
3131
extern void test_client_install (TestSuite *suite);
3232
extern void test_client_pool_install (TestSuite *suite);
33+
extern void test_cluster_install (TestSuite *suite);
3334
extern void test_collection_install (TestSuite *suite);
3435
extern void test_cursor_install (TestSuite *suite);
3536
extern void test_database_install (TestSuite *suite);
@@ -495,6 +496,7 @@ main (int argc,
495496
test_buffer_install (&suite);
496497
test_client_install (&suite);
497498
test_client_pool_install (&suite);
499+
test_cluster_install (&suite);
498500
test_write_command_install (&suite);
499501
test_bulk_install (&suite);
500502
test_collection_install (&suite);

tests/test-mongoc-cluster.c

Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
#include <mongoc-cluster-private.h>
2+
#include "mongoc-cluster-private.h"
3+
#include "mongoc-client-private.h"
4+
5+
#include "TestSuite.h"
6+
#include "test-libmongoc.h"
7+
8+
9+
#undef MONGOC_LOG_DOMAIN
10+
#define MONGOC_LOG_DOMAIN "cluster-test"
11+
12+
13+
void
14+
call_ismaster (bson_t *reply)
15+
{
16+
bson_t ismaster = BSON_INITIALIZER;
17+
mongoc_client_t *client;
18+
bool r;
19+
20+
BSON_APPEND_INT32 (&ismaster, "isMaster", 1);
21+
client = test_framework_client_new (NULL);
22+
r = mongoc_client_command_simple (client, "admin", &ismaster,
23+
NULL, reply, NULL);
24+
25+
assert (r);
26+
}
27+
28+
29+
char *
30+
set_name (bson_t *ismaster_response)
31+
{
32+
bson_iter_t iter;
33+
34+
if (bson_iter_init_find (&iter, ismaster_response, "setName")) {
35+
return bson_strdup (bson_iter_utf8 (&iter, NULL));
36+
} else {
37+
return NULL;
38+
}
39+
}
40+
41+
42+
int
43+
get_n_members (bson_t *ismaster_response)
44+
{
45+
bson_iter_t iter;
46+
bson_iter_t hosts_iter;
47+
char *name;
48+
int n;
49+
50+
if ((name = set_name (ismaster_response))) {
51+
bson_free (name);
52+
assert (bson_iter_init_find (&iter, ismaster_response, "hosts"));
53+
bson_iter_recurse (&iter, &hosts_iter);
54+
n = 0;
55+
while (bson_iter_next (&hosts_iter)) n++;
56+
return n;
57+
} else {
58+
return 1;
59+
}
60+
}
61+
62+
63+
#define BAD_HOST "mongodb.com:12345"
64+
65+
66+
/* a uri with one bogus host */
67+
mongoc_uri_t *
68+
uri_from_ismaster_plus_one (bson_t *ismaster_response)
69+
{
70+
/* start with one bad host and a comma */
71+
bson_string_t *uri_str = bson_string_new ("mongodb://" BAD_HOST ",");
72+
char *name;
73+
bson_iter_t iter;
74+
bson_iter_t hosts_iter;
75+
76+
if ((name = set_name (ismaster_response))) {
77+
bson_iter_init_find (&iter, ismaster_response, "hosts");
78+
bson_iter_recurse (&iter, &hosts_iter);
79+
while (bson_iter_next (&hosts_iter)) {
80+
assert (BSON_ITER_HOLDS_UTF8 (&hosts_iter));
81+
bson_string_append (uri_str, bson_iter_utf8 (&hosts_iter, NULL));
82+
while (bson_iter_next (&hosts_iter)) {
83+
bson_string_append (uri_str, ",");
84+
bson_string_append (uri_str, bson_iter_utf8 (&hosts_iter, NULL));
85+
}
86+
87+
bson_string_append_printf (
88+
uri_str, "/?replicaSet=%s&connecttimeoutms=1000", name);
89+
}
90+
91+
bson_free (name);
92+
} else {
93+
char *host = test_framework_get_host ();
94+
95+
bson_string_append (uri_str, host);
96+
bson_string_append (uri_str, "/?connecttimeoutms=1000");
97+
98+
bson_free (host);
99+
}
100+
101+
return mongoc_uri_new (bson_string_free (uri_str, false));
102+
}
103+
104+
105+
bool
106+
cluster_has_host (const mongoc_cluster_t *cluster,
107+
const char *host_and_port)
108+
{
109+
int i;
110+
111+
for (i = 0; i < cluster->nodes_len; i++) {
112+
if (!strcmp(cluster->nodes[i].host.host_and_port, host_and_port)) {
113+
return true;
114+
}
115+
}
116+
117+
return false;
118+
}
119+
120+
121+
int
122+
hosts_len (const mongoc_host_list_t *hl)
123+
{
124+
int n = 0;
125+
126+
if (!hl) {
127+
return 0;
128+
}
129+
130+
do { n++; } while ((hl = hl->next));
131+
132+
return n;
133+
}
134+
135+
136+
void
137+
assert_hosts_equal (const mongoc_host_list_t *hl,
138+
const mongoc_cluster_t *cluster)
139+
{
140+
ASSERT_CMPINT (hosts_len (hl), ==, cluster->nodes_len);
141+
142+
while (hl) {
143+
if (!cluster_has_host (cluster, hl->host_and_port) &&
144+
strcmp (BAD_HOST, hl->host_and_port)) {
145+
printf ("cluster has no host %s\n", hl->host_and_port);
146+
abort ();
147+
}
148+
149+
hl = hl->next;
150+
}
151+
}
152+
153+
154+
/* not very exhaustive, but ensure that cluster reflects whatever server
155+
* we're connected to */
156+
static void
157+
test_mongoc_cluster_basic (void)
158+
{
159+
mongoc_client_t *client;
160+
bson_t reply;
161+
mongoc_uri_t *uri;
162+
const mongoc_host_list_t *hosts;
163+
bson_error_t error;
164+
int n_members;
165+
char *replica_set_name;
166+
int i;
167+
int n;
168+
169+
call_ismaster (&reply);
170+
n_members = get_n_members (&reply);
171+
replica_set_name = set_name (&reply);
172+
uri = uri_from_ismaster_plus_one (&reply);
173+
174+
/* get hosts list from uri */
175+
hosts = mongoc_uri_get_hosts (uri);
176+
assert (hosts);
177+
ASSERT_CMPSTR (BAD_HOST, hosts->host_and_port);
178+
179+
if (replica_set_name) {
180+
/* remove bad host we prepended, because the cluster will remove it
181+
* once it finds the primary */
182+
hosts = hosts->next;
183+
assert (hosts);
184+
}
185+
186+
client = mongoc_client_new_from_uri (uri);
187+
188+
ASSERT_CMPINT (n_members, ==, client->cluster.nodes_len - 1);
189+
if (replica_set_name) {
190+
ASSERT_CMPINT (MONGOC_CLUSTER_REPLICA_SET, ==, client->cluster.mode);
191+
} else {
192+
/* sharded mode, since we gave 2 seeds */
193+
ASSERT_CMPINT (MONGOC_CLUSTER_SHARDED_CLUSTER, ==, client->cluster.mode);
194+
}
195+
196+
/* connect twice and assert cluster nodes are as expected */
197+
for (i = 0; i < 2; i++) {
198+
/* warnings about failing to connect to mongodb.com:12345 */
199+
suppress_one_message ();
200+
suppress_one_message ();
201+
suppress_one_message ();
202+
203+
_mongoc_cluster_reconnect (&client->cluster, &error);
204+
205+
assert_hosts_equal (hosts, &client->cluster);
206+
207+
for (n = 0; n < client->cluster.nodes_len; n++) {
208+
const mongoc_cluster_node_t *node = &client->cluster.nodes[n];
209+
bool valid_host;
210+
211+
assert (node->valid);
212+
213+
valid_host = (strlen (node->host.host_and_port) &&
214+
strcmp (node->host.host_and_port, BAD_HOST));
215+
216+
if (valid_host) {
217+
assert (node->stream);
218+
} else {
219+
assert (!node->stream);
220+
}
221+
222+
ASSERT_CMPINT (n, ==, node->index);
223+
ASSERT_CMPINT (0, ==, node->stamp);
224+
ASSERT_CMPSTR (replica_set_name, node->replSet);
225+
}
226+
}
227+
228+
bson_free (replica_set_name);
229+
mongoc_uri_destroy (uri);
230+
bson_destroy (&reply);
231+
mongoc_client_destroy (client);
232+
}
233+
234+
235+
void
236+
test_cluster_install (TestSuite *suite)
237+
{
238+
TestSuite_Add (suite, "/Cluster/basic", test_mongoc_cluster_basic);
239+
}

0 commit comments

Comments
 (0)