From 16e8d03c7e4afa4c0e186f8ea50389fc3735e879 Mon Sep 17 00:00:00 2001 From: Jeff Darcy Date: Tue, 25 Jul 2017 17:40:49 -0700 Subject: glusterd: make peerfile parsing more robust Differential Revision: https://phabricator.intern.facebook.com/D5498639 Change-Id: I3184ed8f3dadbdcffd46f4ade855fa93131efa82 BUG: 1462969 Signed-off-by: Jeff Darcy Reviewed-on: https://review.gluster.org/17885 Smoke: Gluster Build System Tested-by: Jeff Darcy Reviewed-by: Prashanth Pai CentOS-regression: Gluster Build System Reviewed-by: Atin Mukherjee --- tests/basic/peer-parsing.t | 52 ++++++++++++++++++++++++++++++ xlators/mgmt/glusterd/src/glusterd-store.c | 52 ++++++++++++++++++++++-------- 2 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 tests/basic/peer-parsing.t diff --git a/tests/basic/peer-parsing.t b/tests/basic/peer-parsing.t new file mode 100644 index 00000000000..210ec8ccd90 --- /dev/null +++ b/tests/basic/peer-parsing.t @@ -0,0 +1,52 @@ +#!/bin/bash + +. $(dirname $0)/../include.rc + +PEER_DIR=/var/lib/glusterd/peers +TEST mkdir -p $PEER_DIR + +declare -i HOST_NUM=100 + +create_random_peer_files() { + for i in $(seq 0 9); do + local peer_uuid=$(uuidgen) + # The rules for quoting and variable substitution in + # here documents would force this to be even less + # readable that way. + ( + echo "state=1" + echo "uuid=$peer_uuid" + echo "hostname=127.0.0.$HOST_NUM" + ) > $PEER_DIR/$peer_uuid + HOST_NUM+=1 + done +} + +create_non_peer_file() { + echo "random stuff" > $PEER_DIR/not_a_peer_file +} + +create_malformed_peer_file() { + echo "more random stuff" > $PEER_DIR/$(uuidgen) +} + +# We create lots of files, in batches, to ensure that our bogus ones are +# properly interspersed with the valid ones. + +TEST create_random_peer_files +TEST create_non_peer_file +TEST create_random_peer_files +TEST create_malformed_peer_file +TEST create_random_peer_files + +# There should be 30 peers, not counting the two bogus files. +TEST glusterd +N_PEERS=$($CLI peer status | grep ^Uuid: | wc -l) +TEST [ "$N_PEERS" = "30" ] + +# For extra credit, check the logs for messages about bogus files. + +cleanup + + + diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c index 8eb301f040f..2dc35174f95 100644 --- a/xlators/mgmt/glusterd/src/glusterd-store.c +++ b/xlators/mgmt/glusterd/src/glusterd-store.c @@ -4293,6 +4293,8 @@ glusterd_store_retrieve_peers (xlator_t *this) glusterd_peerctx_args_t args = {0}; gf_store_op_errno_t op_errno = GD_STORE_SUCCESS; glusterd_peer_hostname_t *address = NULL; + uuid_t tmp_uuid; + gf_boolean_t is_ok; GF_ASSERT (this); priv = this->private; @@ -4312,21 +4314,29 @@ glusterd_store_retrieve_peers (xlator_t *this) goto out; } - GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch); - - while (entry) { + for (;;) { + GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch); + if (!entry) { + break; + } + if (gf_uuid_parse (entry->d_name, tmp_uuid) != 0) { + gf_log (this->name, GF_LOG_WARNING, + "skipping non-peer file %s", entry->d_name); + continue; + } + is_ok = _gf_false; snprintf (filepath, PATH_MAX, "%s/%s", path, entry->d_name); ret = gf_store_handle_retrieve (filepath, &shandle); if (ret) - goto out; + goto next; ret = gf_store_iter_new (shandle, &iter); if (ret) - goto out; + goto next; ret = gf_store_iter_get_next (iter, &key, &value, &op_errno); if (ret) - goto out; + goto next; /* Create an empty peerinfo object before reading in the * details @@ -4335,7 +4345,7 @@ glusterd_store_retrieve_peers (xlator_t *this) NULL, 0); if (peerinfo == NULL) { ret = -1; - goto out; + goto next; } while (!ret) { @@ -4367,11 +4377,18 @@ glusterd_store_retrieve_peers (xlator_t *this) &op_errno); } if (op_errno != GD_STORE_EOF) { - goto out; + goto next; } (void) gf_store_iter_destroy (iter); + if (gf_uuid_is_null (peerinfo->uuid)) { + gf_log ("", GF_LOG_ERROR, + "Null UUID while attempting to read peer from '%s'", + filepath); + goto next; + } + /* Set first hostname from peerinfo->hostnames to * peerinfo->hostname */ @@ -4380,17 +4397,27 @@ glusterd_store_retrieve_peers (xlator_t *this) hostname_list); if (!address) { ret = -1; - goto out; + goto next; } peerinfo->hostname = gf_strdup (address->hostname); ret = glusterd_friend_add_from_peerinfo (peerinfo, 1, NULL); if (ret) - goto out; + goto next; peerinfo->shandle = shandle; + is_ok = _gf_true; + +next: + if (!is_ok) { + gf_log (this->name, GF_LOG_WARNING, + "skipping malformed peer file %s", + entry->d_name); + if (peerinfo) { + glusterd_peerinfo_cleanup (peerinfo); + } + } peerinfo = NULL; - GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch); } args.mode = GD_MODE_ON; @@ -4405,9 +4432,6 @@ glusterd_store_retrieve_peers (xlator_t *this) peerinfo = NULL; out: - if (peerinfo) - glusterd_peerinfo_cleanup (peerinfo); - if (dir) sys_closedir (dir); gf_msg_debug (this->name, 0, "Returning with %d", ret); -- cgit