summaryrefslogtreecommitdiffstats
path: root/xlators/cluster
diff options
context:
space:
mode:
authorRaghavendra G <rgowdapp@redhat.com>2016-11-08 12:09:57 +0530
committerShyamsundar Ranganathan <srangana@redhat.com>2016-12-08 09:56:41 -0800
commit64451d0f25e7cc7aafc1b6589122648281e4310a (patch)
treec3eba861ae8124ac20217927cf0719ec5d3d0b18 /xlators/cluster
parent58a58e706da73ee751b7cd98c23e6675667fefdb (diff)
cluster/dht: Fix memory corruption while accessing regex stored in
private If reconfigure is executed parallely (or concurrently with dht_init), there are races that can corrupt memory. One such race is modification of regexes stored in conf (conf->rsync_regex_valid and conf->extra_regex_valid) through dht_init_regex. With change [1], reconfigure codepath can get executed parallely (with itself or with dht_init) and this fix is needed. Also, a reconfigure can race with any thread doing dht_layout_search, resulting in dht_layout_search accessing regex freed up by reconfigure (like in bz 1399134). [1] http://review.gluster.org/15046 Change-Id: I039422a65374cf0ccbe0073441f0e8c442ebf830 BUG: 1399134 Signed-off-by: Raghavendra G <rgowdapp@redhat.com> Reviewed-on: http://review.gluster.org/15945 Smoke: Gluster Build System <jenkins@build.gluster.org> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> Reviewed-by: N Balachandran <nbalacha@redhat.com> CentOS-regression: Gluster Build System <jenkins@build.gluster.org> Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>
Diffstat (limited to 'xlators/cluster')
-rw-r--r--xlators/cluster/dht/src/dht-common.h1
-rw-r--r--xlators/cluster/dht/src/dht-hashfn.c54
-rw-r--r--xlators/cluster/dht/src/dht-shared.c54
3 files changed, 64 insertions, 45 deletions
diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h
index 719d214f92d..be9d2ce5e4e 100644
--- a/xlators/cluster/dht/src/dht-common.h
+++ b/xlators/cluster/dht/src/dht-common.h
@@ -580,6 +580,7 @@ struct dht_conf {
/* lock migration */
gf_boolean_t lock_migration_enabled;
+ gf_lock_t lock;
};
typedef struct dht_conf dht_conf_t;
diff --git a/xlators/cluster/dht/src/dht-hashfn.c b/xlators/cluster/dht/src/dht-hashfn.c
index 66e3ede736b..f8e614a40aa 100644
--- a/xlators/cluster/dht/src/dht-hashfn.c
+++ b/xlators/cluster/dht/src/dht-hashfn.c
@@ -41,12 +41,16 @@ dht_hash_compute_internal (int type, const char *name, uint32_t *hash_p)
static
gf_boolean_t
-dht_munge_name (const char *original, char *modified, size_t len, regex_t *re)
+dht_munge_name (const char *original, char *modified,
+ size_t len, regex_t *re)
{
- regmatch_t matches[2];
- size_t new_len;
+ regmatch_t matches[2] = {{0}, };
+ size_t new_len = 0;
+ int ret = 0;
- if (regexec(re,original,2,matches,0) != REG_NOMATCH) {
+ ret = regexec(re, original, 2, matches, 0);
+
+ if (ret != REG_NOMATCH) {
if (matches[1].rm_so != -1) {
new_len = matches[1].rm_eo - matches[1].rm_so;
/* Equal would fail due to the NUL at the end. */
@@ -60,7 +64,7 @@ dht_munge_name (const char *original, char *modified, size_t len, regex_t *re)
}
/* This is guaranteed safe because of how the dest was allocated. */
- strcpy(modified,original);
+ strcpy(modified, original);
return _gf_false;
}
@@ -68,28 +72,36 @@ int
dht_hash_compute (xlator_t *this, int type, const char *name, uint32_t *hash_p)
{
char *rsync_friendly_name = NULL;
- dht_conf_t *priv = this->private;
+ dht_conf_t *priv = NULL;
size_t len = 0;
gf_boolean_t munged = _gf_false;
- if (priv->extra_regex_valid) {
- len = strlen(name) + 1;
- rsync_friendly_name = alloca(len);
- munged = dht_munge_name (name, rsync_friendly_name, len,
- &priv->extra_regex);
- }
+ priv = this->private;
- if (!munged && priv->rsync_regex_valid) {
- len = strlen(name) + 1;
- rsync_friendly_name = alloca(len);
- gf_msg_trace (this->name, 0, "trying regex for %s", name);
- munged = dht_munge_name (name, rsync_friendly_name, len,
- &priv->rsync_regex);
- if (munged) {
- gf_msg_debug (this->name, 0,
- "munged down to %s", rsync_friendly_name);
+ LOCK (&priv->lock);
+ {
+ if (priv->extra_regex_valid) {
+ len = strlen(name) + 1;
+ rsync_friendly_name = alloca(len);
+ munged = dht_munge_name (name, rsync_friendly_name, len,
+ &priv->extra_regex);
+ }
+
+ if (!munged && priv->rsync_regex_valid) {
+ len = strlen(name) + 1;
+ rsync_friendly_name = alloca(len);
+ gf_msg_trace (this->name, 0, "trying regex for %s",
+ name);
+ munged = dht_munge_name (name, rsync_friendly_name, len,
+ &priv->rsync_regex);
+ if (munged) {
+ gf_msg_debug (this->name, 0,
+ "munged down to %s",
+ rsync_friendly_name);
+ }
}
}
+ UNLOCK (&priv->lock);
if (!munged) {
rsync_friendly_name = (char *)name;
diff --git a/xlators/cluster/dht/src/dht-shared.c b/xlators/cluster/dht/src/dht-shared.c
index 46bf461cf63..f13762e34fb 100644
--- a/xlators/cluster/dht/src/dht-shared.c
+++ b/xlators/cluster/dht/src/dht-shared.c
@@ -336,9 +336,9 @@ out:
}
void
dht_init_regex (xlator_t *this, dict_t *odict, char *name,
- regex_t *re, gf_boolean_t *re_valid)
+ regex_t *re, gf_boolean_t *re_valid, dht_conf_t *conf)
{
- char *temp_str;
+ char *temp_str = NULL;
if (dict_get_str (odict, name, &temp_str) != 0) {
if (strcmp(name,"rsync-hash-regex")) {
@@ -347,25 +347,29 @@ dht_init_regex (xlator_t *this, dict_t *odict, char *name,
temp_str = "^\\.(.+)\\.[^.]+$";
}
- if (*re_valid) {
- regfree(re);
- *re_valid = _gf_false;
- }
+ LOCK (&conf->lock);
+ {
+ if (*re_valid) {
+ regfree(re);
+ *re_valid = _gf_false;
+ }
- if (!strcmp(temp_str,"none")) {
- return;
- }
+ if (!strcmp(temp_str, "none")) {
+ goto unlock;
+ }
- if (regcomp(re,temp_str,REG_EXTENDED) == 0) {
- gf_msg_debug (this->name, 0,
- "using regex %s = %s", name, temp_str);
- *re_valid = _gf_true;
- }
- else {
- gf_msg (this->name, GF_LOG_WARNING, 0,
- DHT_MSG_REGEX_INFO,
- "compiling regex %s failed", temp_str);
+ if (regcomp(re, temp_str, REG_EXTENDED) == 0) {
+ gf_msg_debug (this->name, 0,
+ "using regex %s = %s", name, temp_str);
+ *re_valid = _gf_true;
+ } else {
+ gf_msg (this->name, GF_LOG_WARNING, 0,
+ DHT_MSG_REGEX_INFO,
+ "compiling regex %s failed", temp_str);
+ }
}
+unlock:
+ UNLOCK (&conf->lock);
}
int
@@ -486,9 +490,9 @@ dht_reconfigure (xlator_t *this, dict_t *options)
}
dht_init_regex (this, options, "rsync-hash-regex",
- &conf->rsync_regex, &conf->rsync_regex_valid);
+ &conf->rsync_regex, &conf->rsync_regex_valid, conf);
dht_init_regex (this, options, "extra-hash-regex",
- &conf->extra_regex, &conf->extra_regex_valid);
+ &conf->extra_regex, &conf->extra_regex_valid, conf);
GF_OPTION_RECONF ("weighted-rebalance", conf->do_weighting, options,
bool, out);
@@ -627,6 +631,10 @@ dht_init (xlator_t *this)
goto err;
}
+ LOCK_INIT (&conf->subvolume_lock);
+ LOCK_INIT (&conf->layout_lock);
+ LOCK_INIT (&conf->lock);
+
/* We get the commit-hash to set only for rebalance process */
if (dict_get_uint32 (this->options,
"commit-hash", &commit_hash) == 0) {
@@ -776,17 +784,15 @@ dht_init (xlator_t *this)
}
dht_init_regex (this, this->options, "rsync-hash-regex",
- &conf->rsync_regex, &conf->rsync_regex_valid);
+ &conf->rsync_regex, &conf->rsync_regex_valid, conf);
dht_init_regex (this, this->options, "extra-hash-regex",
- &conf->extra_regex, &conf->extra_regex_valid);
+ &conf->extra_regex, &conf->extra_regex_valid, conf);
ret = dht_layouts_init (this, conf);
if (ret == -1) {
goto err;
}
- LOCK_INIT (&conf->subvolume_lock);
- LOCK_INIT (&conf->layout_lock);
conf->gen = 1;