diff options
| author | ShyamsundarR <srangana@redhat.com> | 2018-08-22 15:04:52 -0400 |
|---|---|---|
| committer | N Balachandran <nbalacha@redhat.com> | 2018-08-24 05:11:11 +0000 |
| commit | 2785d2fdf710fcd323bef2d1cbbf17fe823a289f (patch) | |
| tree | f96da2adbf6f40e544b34cf5131321c15ca7a7c4 /xlators/cluster/dht | |
| parent | f191bb7bc1abd250bdf0a5a6972ce95fcbd3314b (diff) | |
dht/switch: Fix coverity issues
CID: 1124779: dup_childs not freed in an err path
CID: 1382398: option_string leaked
CID: 1382389: memcpy may cause string overflow
Also, fixed up NULL termination for the string
Potential use after free (or double free in this case) as below,
(link split into multiple lines)
https://download.gluster.org/pub/gluster/glusterfs/static-analysis/
master/glusterfs-coverity/2018-08-22-0ebaa9c6/html/
1/427switch.c.html#error
Change-Id: I76681af6a8091666918a3d5dff30a152a7b97905
Updates: bz#789278
Signed-off-by: ShyamsundarR <srangana@redhat.com>
Diffstat (limited to 'xlators/cluster/dht')
| -rw-r--r-- | xlators/cluster/dht/src/switch.c | 23 |
1 files changed, 19 insertions, 4 deletions
diff --git a/xlators/cluster/dht/src/switch.c b/xlators/cluster/dht/src/switch.c index 9795ff4af4d..8b66a09b822 100644 --- a/xlators/cluster/dht/src/switch.c +++ b/xlators/cluster/dht/src/switch.c @@ -617,9 +617,10 @@ set_switch_pattern (xlator_t *this, dht_conf_t *conf, char *pattern = NULL; char *childs = NULL; char *option_string = NULL; - struct switch_struct *switch_buf = NULL; - struct switch_struct *switch_opt = NULL; - struct switch_struct *trav = NULL; + size_t pattern_length; + struct switch_struct *switch_buf = NULL; + struct switch_struct *switch_opt = NULL; + struct switch_struct *trav = NULL; struct switch_sched_array *switch_buf_array = NULL; xlator_list_t *trav_xl = NULL; @@ -670,11 +671,22 @@ set_switch_pattern (xlator_t *this, dht_conf_t *conf, " hence neglecting current option"); switch_str = strtok_r (NULL, ";", &tmp_str); GF_FREE (switch_opt); + switch_opt = NULL; GF_FREE (dup_str); continue; } GF_FREE (dup_str); - memcpy (switch_opt->path_pattern, pattern, strlen (pattern)); + + pattern_length = strlen (pattern); + if (pattern_length >= (sizeof (switch_opt->path_pattern))) { + gf_msg (this->name, GF_LOG_ERROR, 0, + DHT_MSG_SET_SWITCH_PATTERN_ERROR, + "Pattern (%s) too long", pattern); + goto err; + } + memcpy (switch_opt->path_pattern, pattern, pattern_length); + switch_opt->path_pattern[pattern_length] = '\0'; + if (childs) { dup_childs = gf_strdup (childs); child = strtok_r (dup_childs, ",", &tmp); @@ -689,6 +701,7 @@ set_switch_pattern (xlator_t *this, dht_conf_t *conf, "pattern can only be scheduled " "only to a subvolume of %s", child, this->name, this->name); + GF_FREE (dup_childs); goto err; } } @@ -808,10 +821,12 @@ set_switch_pattern (xlator_t *this, dht_conf_t *conf, /* */ conf->private = switch_buf; + GF_FREE (option_string); return 0; err: GF_FREE (switch_buf_array); GF_FREE (switch_opt); + GF_FREE (option_string); if (switch_buf) { trav = switch_buf; |