diff options
author | N Balachandran <nbalacha@redhat.com> | 2018-10-17 18:48:19 +0530 |
---|---|---|
committer | Shyamsundar Ranganathan <srangana@redhat.com> | 2018-10-17 23:42:56 +0000 |
commit | 219cd649fdbd7bfd6c2268a0a4f66bcc15918e31 (patch) | |
tree | 3453c0102cbc975b92b10266961d76250b5d002c | |
parent | 6257276d9de3f15643f159b2ec627a67c84fc23d (diff) |
debug/io-stats: io stats filenames contain garbage
As dict_unserialize does not null terminate the value,
using snprintf adds garbage characters to the buffer
used to create the filename.
The code also used this->name in the filename which
will be the same for all bricks for a volume. The
files were thus overwritten if a node contained
multiple bricks for a volume. The code now uses
the conf->unique instead if available.
Change-Id: I2c72534b32634b87961d3b3f7d53c5f2ca2c068c
fixes: bz#1640165
Signed-off-by: N Balachandran <nbalacha@redhat.com>
-rw-r--r-- | xlators/debug/io-stats/src/io-stats.c | 27 |
1 files changed, 23 insertions, 4 deletions
diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c index 3c7e7cf1ea4..f2b72369421 100644 --- a/xlators/debug/io-stats/src/io-stats.c +++ b/xlators/debug/io-stats/src/io-stats.c @@ -2923,9 +2923,12 @@ conditional_dump(dict_t *dict, char *key, data_t *value, void *data) char dump_key[100]; char *slash_ptr = NULL; char *path_in_value = NULL; + char *identifier = NULL; + struct ios_conf *conf = NULL; stub = data; this = stub->this; + conf = this->private; /* Create a file name that is appended with the io-stats instance name as well. This helps when there is more than a single io-stats @@ -2938,21 +2941,37 @@ conditional_dump(dict_t *dict, char *key, data_t *value, void *data) /* name format: /var/run/gluster/<passed in path/filename>.<xlator name * slashes to -> */ - path_in_value = data_to_str(value); + path_in_value = alloca0(value->len + 1); + + /* We need a memcpy here because of the way dict_unserialize works */ + + memcpy(path_in_value, data_to_str(value), value->len); + path_in_value[value->len] = '\0'; if (strstr(path_in_value, "../")) { gf_log(this->name, GF_LOG_ERROR, "%s: no \"../\" allowed in path", path_in_value); return -1; } + + if (path_in_value[0] == '/') { + path_in_value = path_in_value + 1; + } + dirlen = strlen(IOS_STATS_DUMP_DIR); - namelen = (dirlen + value->len + strlen(this->name) + 3); + if (conf->unique_id) { + /* this->name will be the same for all bricks of the volume */ + identifier = conf->unique_id; + } else { + identifier = this->name; + } + + namelen = (dirlen + value->len + strlen(identifier) + 3); /* +3 for '/', '.' and '\0' added in snprintf below*/ filename = alloca0(namelen); - snprintf(filename, namelen, "%s/%s.%s", IOS_STATS_DUMP_DIR, path_in_value, - this->name); + identifier); /* convert any slashes to '-' so that fopen works correctly */ slash_ptr = strchr(filename + dirlen + 1, '/'); |