summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorXavi Hernandez <xhernandez@redhat.com>2018-03-09 22:48:33 +0100
committerXavi Hernandez <xhernandez@redhat.com>2018-03-09 23:31:29 +0100
commit157e55fe43ba13f04452aa11f42200b279fb4f7a (patch)
tree8ea7ab1685741b8236fde8a7accc611e98d73acc
parent940f870f4716f9cd32c68db95aa326a0ae87bf03 (diff)
protocol/client: fix memory corruption
There was an issue when some accesses to saved_fds list were protected by the wrong mutex (lock instead of fd_lock). Additionally, the retrieval of fdctx from fd's context and any checks done on it have also been protected by fd_lock to avoid fdctx to become outdated just after retrieving it. Change-Id: If2910508bcb7d1ff23debb30291391f00903a6fe BUG: 1553129 Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
-rw-r--r--xlators/protocol/client/src/client-handshake.c9
-rw-r--r--xlators/protocol/client/src/client-helpers.c34
-rw-r--r--xlators/protocol/client/src/client-lk.c94
-rw-r--r--xlators/protocol/client/src/client-rpc-fops.c24
-rw-r--r--xlators/protocol/client/src/client-rpc-fops_v2.c8
-rw-r--r--xlators/protocol/client/src/client.h1
6 files changed, 78 insertions, 92 deletions
diff --git a/xlators/protocol/client/src/client-handshake.c b/xlators/protocol/client/src/client-handshake.c
index 74c601bbcbd..5c0b4750e2e 100644
--- a/xlators/protocol/client/src/client-handshake.c
+++ b/xlators/protocol/client/src/client-handshake.c
@@ -926,11 +926,14 @@ client_attempt_reopen (fd_t *fd, xlator_t *this)
conf = this->private;
- fdctx = this_fd_get_ctx (fd, this);
- if (!fdctx)
- goto out;
pthread_spin_lock (&conf->fd_lock);
{
+ fdctx = this_fd_get_ctx (fd, this);
+ if (!fdctx) {
+ pthread_spin_unlock(&conf->fd_lock);
+ goto out;
+ }
+
if (__is_fd_reopen_in_progress (fdctx))
goto unlock;
if (fdctx->remote_fd != -1)
diff --git a/xlators/protocol/client/src/client-helpers.c b/xlators/protocol/client/src/client-helpers.c
index 465de2a52d4..d0ccca73fe5 100644
--- a/xlators/protocol/client/src/client-helpers.c
+++ b/xlators/protocol/client/src/client-helpers.c
@@ -424,20 +424,20 @@ client_get_remote_fd (xlator_t *this, fd_t *fd, int flags, int64_t *remote_fd)
GF_VALIDATE_OR_GOTO (this->name, fd, out);
GF_VALIDATE_OR_GOTO (this->name, remote_fd, out);
- fdctx = this_fd_get_ctx (fd, this);
- if (!fdctx) {
- *remote_fd = GF_ANON_FD_NO;
- } else {
- conf = this->private;
- pthread_spin_lock (&conf->fd_lock);
- {
+ conf = this->private;
+ pthread_spin_lock (&conf->fd_lock);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ if (!fdctx) {
+ *remote_fd = GF_ANON_FD_NO;
+ } else {
if (__is_fd_reopen_in_progress (fdctx))
*remote_fd = -1;
else
*remote_fd = fdctx->remote_fd;
}
- pthread_spin_unlock (&conf->fd_lock);
}
+ pthread_spin_unlock (&conf->fd_lock);
if ((flags & FALLBACK_TO_ANON_FD) && (*remote_fd == -1))
*remote_fd = GF_ANON_FD_NO;
@@ -450,13 +450,21 @@ out:
gf_boolean_t
client_is_reopen_needed (fd_t *fd, xlator_t *this, int64_t remote_fd)
{
+ clnt_conf_t *conf = NULL;
clnt_fd_ctx_t *fdctx = NULL;
+ gf_boolean_t res = _gf_false;
+
+ conf = this->private;
+ pthread_spin_lock(&conf->fd_lock);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ if (fdctx && (fdctx->remote_fd == -1) &&
+ (remote_fd == GF_ANON_FD_NO))
+ res = _gf_true;
+ }
+ pthread_spin_unlock(&conf->fd_lock);
- fdctx = this_fd_get_ctx (fd, this);
- if (fdctx && (fdctx->remote_fd == -1) &&
- (remote_fd == GF_ANON_FD_NO))
- return _gf_true;
- return _gf_false;
+ return res;
}
int
diff --git a/xlators/protocol/client/src/client-lk.c b/xlators/protocol/client/src/client-lk.c
index 6468fb4511e..b5e11aa07fd 100644
--- a/xlators/protocol/client/src/client-lk.c
+++ b/xlators/protocol/client/src/client-lk.c
@@ -43,14 +43,10 @@ dump_client_locks_fd (clnt_fd_ctx_t *fdctx)
client_posix_lock_t *lock = NULL;
int count = 0;
- pthread_mutex_lock (&fdctx->mutex);
- {
- list_for_each_entry (lock, &fdctx->lock_list, list) {
- __dump_client_lock (lock);
- count++;
- }
+ list_for_each_entry (lock, &fdctx->lock_list, list) {
+ __dump_client_lock (lock);
+ count++;
}
- pthread_mutex_unlock (&fdctx->mutex);
return count;
@@ -59,23 +55,27 @@ dump_client_locks_fd (clnt_fd_ctx_t *fdctx)
int
dump_client_locks (inode_t *inode)
{
- fd_t *fd = NULL;
- xlator_t *this = NULL;
- clnt_fd_ctx_t *fdctx = NULL;
+ fd_t *fd = NULL;
+ xlator_t *this = NULL;
+ clnt_fd_ctx_t *fdctx = NULL;
+ clnt_conf_t *conf = NULL;
int total_count = 0;
int locks_fd_count = 0;
this = THIS;
+ conf = this->private;
LOCK (&inode->lock);
{
list_for_each_entry (fd, &inode->fd_list, inode_list) {
locks_fd_count = 0;
+ pthread_spin_lock(&conf->fd_lock);
fdctx = this_fd_get_ctx (fd, this);
if (fdctx)
locks_fd_count = dump_client_locks_fd (fdctx);
+ pthread_spin_unlock(&conf->fd_lock);
total_count += locks_fd_count;
}
@@ -327,13 +327,7 @@ __insert_and_merge (clnt_fd_ctx_t *fdctx, client_posix_lock_t *lock)
static void
client_setlk (clnt_fd_ctx_t *fdctx, client_posix_lock_t *lock)
{
- pthread_mutex_lock (&fdctx->mutex);
- {
- __insert_and_merge (fdctx, lock);
- }
- pthread_mutex_unlock (&fdctx->mutex);
-
- return;
+ __insert_and_merge (fdctx, lock);
}
static void
@@ -349,6 +343,7 @@ delete_granted_locks_owner (fd_t *fd, gf_lkowner_t *owner)
client_posix_lock_t *lock = NULL;
client_posix_lock_t *tmp = NULL;
xlator_t *this = NULL;
+ clnt_conf_t *conf = NULL;
struct list_head delete_list;
int ret = 0;
@@ -356,25 +351,29 @@ delete_granted_locks_owner (fd_t *fd, gf_lkowner_t *owner)
INIT_LIST_HEAD (&delete_list);
this = THIS;
+ conf = this->private;
+
+ pthread_spin_lock(&conf->fd_lock);
+
fdctx = this_fd_get_ctx (fd, this);
if (!fdctx) {
+ pthread_spin_unlock(&conf->fd_lock);
+
gf_msg (this->name, GF_LOG_WARNING, EINVAL,
PC_MSG_FD_CTX_INVALID, "fdctx not valid");
ret = -1;
goto out;
}
- pthread_mutex_lock (&fdctx->mutex);
- {
- list_for_each_entry_safe (lock, tmp, &fdctx->lock_list, list) {
- if (!is_same_lkowner (&lock->owner, owner)) {
- list_del_init (&lock->list);
- list_add_tail (&lock->list, &delete_list);
- count++;
- }
+ list_for_each_entry_safe (lock, tmp, &fdctx->lock_list, list) {
+ if (!is_same_lkowner (&lock->owner, owner)) {
+ list_del_init (&lock->list);
+ list_add_tail (&lock->list, &delete_list);
+ count++;
}
}
- pthread_mutex_unlock (&fdctx->mutex);
+
+ pthread_spin_unlock(&conf->fd_lock);
list_for_each_entry_safe (lock, tmp, &delete_list, list) {
list_del_init (&lock->list);
@@ -390,39 +389,6 @@ out:
}
int32_t
-delete_granted_locks_fd (clnt_fd_ctx_t *fdctx)
-{
- client_posix_lock_t *lock = NULL;
- client_posix_lock_t *tmp = NULL;
- xlator_t *this = NULL;
-
- struct list_head delete_list;
- int ret = 0;
- int count = 0;
-
- INIT_LIST_HEAD (&delete_list);
- this = THIS;
-
- pthread_mutex_lock (&fdctx->mutex);
- {
- list_splice_init (&fdctx->lock_list, &delete_list);
- }
- pthread_mutex_unlock (&fdctx->mutex);
-
- list_for_each_entry_safe (lock, tmp, &delete_list, list) {
- list_del_init (&lock->list);
- count++;
- destroy_client_lock (lock);
- }
-
- /* FIXME: Need to actually print the locks instead of count */
- gf_msg_trace (this->name, 0,
- "Number of locks cleared=%d", count);
-
- return ret;
-}
-
-int32_t
client_cmd_to_gf_cmd (int32_t cmd, int32_t *gf_cmd)
{
int ret = 0;
@@ -497,13 +463,19 @@ client_add_lock_for_recovery (fd_t *fd, struct gf_flock *flock,
clnt_fd_ctx_t *fdctx = NULL;
xlator_t *this = NULL;
client_posix_lock_t *lock = NULL;
+ clnt_conf_t *conf = NULL;
int ret = 0;
this = THIS;
+ conf = this->private;
+
+ pthread_spin_lock(&conf->fd_lock);
fdctx = this_fd_get_ctx (fd, this);
if (!fdctx) {
+ pthread_spin_unlock(&conf->fd_lock);
+
gf_msg (this->name, GF_LOG_WARNING, 0, PC_MSG_FD_GET_FAIL,
"failed to get fd context. sending EBADFD");
ret = -EBADFD;
@@ -512,12 +484,16 @@ client_add_lock_for_recovery (fd_t *fd, struct gf_flock *flock,
lock = new_client_lock (flock, owner, cmd, fd);
if (!lock) {
+ pthread_spin_unlock(&conf->fd_lock);
+
ret = -ENOMEM;
goto out;
}
client_setlk (fdctx, lock);
+ pthread_spin_unlock(&conf->fd_lock);
+
out:
return ret;
diff --git a/xlators/protocol/client/src/client-rpc-fops.c b/xlators/protocol/client/src/client-rpc-fops.c
index 03ca2172692..94fe4ea5ad2 100644
--- a/xlators/protocol/client/src/client-rpc-fops.c
+++ b/xlators/protocol/client/src/client-rpc-fops.c
@@ -368,10 +368,10 @@ client_add_fd_to_saved_fds (xlator_t *this, fd_t *fd, loc_t *loc, int32_t flags,
INIT_LIST_HEAD (&fdctx->sfd_pos);
INIT_LIST_HEAD (&fdctx->lock_list);
- this_fd_set_ctx (fd, this, loc, fdctx);
-
pthread_spin_lock (&conf->fd_lock);
{
+ this_fd_set_ctx (fd, this, loc, fdctx);
+
list_add_tail (&fdctx->sfd_pos, &conf->saved_fds);
}
pthread_spin_unlock (&conf->fd_lock);
@@ -3225,10 +3225,10 @@ client3_3_releasedir (call_frame_t *frame, xlator_t *this,
args = data;
conf = this->private;
- fdctx = this_fd_del_ctx (args->fd, this);
- if (fdctx != NULL) {
- pthread_spin_lock (&conf->fd_lock);
- {
+ pthread_spin_lock (&conf->fd_lock);
+ {
+ fdctx = this_fd_del_ctx (args->fd, this);
+ if (fdctx != NULL) {
remote_fd = fdctx->remote_fd;
/* fdctx->remote_fd == -1 indicates a reopen attempt
@@ -3243,8 +3243,8 @@ client3_3_releasedir (call_frame_t *frame, xlator_t *this,
destroy = _gf_true;
}
}
- pthread_spin_unlock (&conf->fd_lock);
}
+ pthread_spin_unlock (&conf->fd_lock);
if (destroy)
client_fdctx_destroy (this, fdctx);
@@ -3270,10 +3270,10 @@ client3_3_release (call_frame_t *frame, xlator_t *this,
args = data;
conf = this->private;
- fdctx = this_fd_del_ctx (args->fd, this);
- if (fdctx != NULL) {
- pthread_spin_lock (&conf->fd_lock);
- {
+ pthread_spin_lock (&conf->fd_lock);
+ {
+ fdctx = this_fd_del_ctx (args->fd, this);
+ if (fdctx != NULL) {
remote_fd = fdctx->remote_fd;
/* fdctx->remote_fd == -1 indicates a reopen attempt
@@ -3287,8 +3287,8 @@ client3_3_release (call_frame_t *frame, xlator_t *this,
destroy = _gf_true;
}
}
- pthread_spin_unlock (&conf->fd_lock);
}
+ pthread_spin_unlock (&conf->fd_lock);
if (destroy)
client_fdctx_destroy (this, fdctx);
diff --git a/xlators/protocol/client/src/client-rpc-fops_v2.c b/xlators/protocol/client/src/client-rpc-fops_v2.c
index aad1666096b..15f1c956101 100644
--- a/xlators/protocol/client/src/client-rpc-fops_v2.c
+++ b/xlators/protocol/client/src/client-rpc-fops_v2.c
@@ -2758,7 +2758,7 @@ client4_0_releasedir (call_frame_t *frame, xlator_t *this,
args = data;
conf = this->private;
- pthread_mutex_lock (&conf->lock);
+ pthread_spin_lock (&conf->fd_lock);
{
fdctx = this_fd_del_ctx (args->fd, this);
if (fdctx != NULL) {
@@ -2777,7 +2777,7 @@ client4_0_releasedir (call_frame_t *frame, xlator_t *this,
}
}
}
- pthread_mutex_unlock (&conf->lock);
+ pthread_spin_unlock (&conf->fd_lock);
if (destroy)
client_fdctx_destroy (this, fdctx);
@@ -2803,7 +2803,7 @@ client4_0_release (call_frame_t *frame, xlator_t *this,
args = data;
conf = this->private;
- pthread_mutex_lock (&conf->lock);
+ pthread_spin_lock (&conf->fd_lock);
{
fdctx = this_fd_del_ctx (args->fd, this);
if (fdctx != NULL) {
@@ -2821,7 +2821,7 @@ client4_0_release (call_frame_t *frame, xlator_t *this,
}
}
}
- pthread_mutex_unlock (&conf->lock);
+ pthread_spin_unlock (&conf->fd_lock);
if (destroy)
client_fdctx_destroy (this, fdctx);
diff --git a/xlators/protocol/client/src/client.h b/xlators/protocol/client/src/client.h
index b51607fdc45..c3c8aaec0dc 100644
--- a/xlators/protocol/client/src/client.h
+++ b/xlators/protocol/client/src/client.h
@@ -246,7 +246,6 @@ typedef struct _client_fd_ctx {
char released;
int32_t flags;
fd_lk_ctx_t *lk_ctx;
- pthread_mutex_t mutex;
uuid_t gfid;
void (*reopen_done)(struct _client_fd_ctx*, int64_t rfd, xlator_t *);
struct list_head lock_list; /* List of all granted locks on this fd */