summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnand Avati <avati@gluster.com>2011-07-15 01:05:16 +0000
committerAnand Avati <avati@gluster.com>2011-07-16 12:47:13 -0700
commit1742383787355241d5aaa4b2bdd92d089ef2d508 (patch)
tree3ed16ed357595ec32d8cd2712fac3d2a1085eb50
parentf50e5eb7777ee31701f5d757ffa8de2c238b5e50 (diff)
posix: perform readdir filling in locked region
When two application threads share an open dir fd (DIR *) and issue readdirs, storage/posix will receive separate readdir fops in separate threads in parallel. This has two-fold issues 1. In the following pair of operations - entry = readdir(dir) and strcpy (gf_dirent->name, entry->d_name) @entry is a static buffer in libc which can get reused by another thread to get filled with a longer name. This can cause the second operation to overflow the buffer as the allocation was for the smaller name. 2. In the following pair of operations - seekdir (dir, offset) and entry = readdir(dir) If two threads are executing these sequence in parallel in separate threads, then one of them will end up reading wrong/unexpected entries. It would be sufficient to fix 1. by using readdir_r but that still keeps the second race open. Hence the patch moves all the set of operations to a locked region which solves both races. Signed-off-by: Anand Avati <avati@gluster.com> BUG: 3171 (Crash in server) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=3171
-rw-r--r--xlators/storage/posix/src/posix.c178
1 files changed, 101 insertions, 77 deletions
diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c
index ed241db2869..8e613ff208e 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -3360,6 +3360,97 @@ posix_fentrylk (call_frame_t *frame, xlator_t *this,
}
+int
+__posix_fill_readdir (DIR *dir, off_t off, size_t size, gf_dirent_t *entries,
+ const char *real_path, const char *base_path)
+{
+ off_t in_case = -1;
+ size_t filled = 0;
+ int ret = 0;
+ int count = 0;
+ struct dirent *entry = NULL;
+ int32_t this_size = -1;
+ gf_dirent_t *this_entry = NULL;
+ char hidden_path[PATH_MAX] = {0, };
+ struct stat statbuf = {0, };
+
+ if (!off) {
+ rewinddir (dir);
+ } else {
+ seekdir (dir, off);
+ }
+
+ while (filled <= size) {
+ in_case = telldir (dir);
+
+ if (in_case == -1) {
+ gf_log (THIS->name, GF_LOG_ERROR,
+ "telldir failed on dir=%p: %s",
+ dir, strerror (errno));
+ goto out;
+ }
+
+ errno = 0;
+ entry = readdir (dir);
+
+ if (!entry) {
+ if (errno == EBADF) {
+ gf_log (THIS->name, GF_LOG_WARNING,
+ "readdir failed on dir=%p: %s",
+ dir, strerror (errno));
+ goto out;
+ }
+ break;
+ }
+
+ if ((!strcmp (real_path, base_path))
+ && (!strcmp (entry->d_name, GF_REPLICATE_TRASH_DIR)))
+ continue;
+
+ if ((!strcmp (real_path, base_path))
+ && (!strncmp (GF_HIDDEN_PATH, entry->d_name,
+ strlen (GF_HIDDEN_PATH)))) {
+ snprintf (hidden_path, PATH_MAX, "%s/%s", real_path,
+ entry->d_name);
+ ret = lstat (hidden_path, &statbuf);
+ if (!ret && S_ISDIR (statbuf.st_mode))
+ continue;
+ }
+
+ this_size = max (sizeof (gf_dirent_t),
+ sizeof (gfs3_dirplist))
+ + strlen (entry->d_name) + 1;
+
+ if (this_size + filled > size) {
+ seekdir (dir, in_case);
+ break;
+ }
+
+ this_entry = gf_dirent_for_name (entry->d_name);
+
+ if (!this_entry) {
+ gf_log (THIS->name, GF_LOG_ERROR,
+ "could not create gf_dirent for entry %s: (%s)",
+ entry->d_name, strerror (errno));
+ goto out;
+ }
+ this_entry->d_off = telldir (dir);
+ this_entry->d_ino = entry->d_ino;
+
+ list_add_tail (&this_entry->list, &entries->list);
+
+ filled += this_size;
+ count ++;
+ }
+
+ if ((!readdir (dir) && (errno == 0)))
+ /* Indicate EOF */
+ errno = ENOENT;
+out:
+ return count;
+}
+
+
int32_t
posix_do_readdir (call_frame_t *frame, xlator_t *this,
fd_t *fd, size_t size, off_t off, int whichop)
@@ -3368,15 +3459,10 @@ posix_do_readdir (call_frame_t *frame, xlator_t *this,
struct posix_fd *pfd = NULL;
DIR *dir = NULL;
int ret = -1;
- size_t filled = 0;
int count = 0;
int32_t op_ret = -1;
int32_t op_errno = 0;
- gf_dirent_t *this_entry = NULL;
gf_dirent_t entries;
- struct dirent *entry = NULL;
- off_t in_case = -1;
- int32_t this_size = -1;
char *real_path = NULL;
int real_path_len = -1;
char *entry_path = NULL;
@@ -3385,8 +3471,7 @@ posix_do_readdir (call_frame_t *frame, xlator_t *this,
struct iatt stbuf = {0, };
char base_path[PATH_MAX] = {0,};
gf_dirent_t *tmp_entry = NULL;
- struct stat statbuf = {0, };
- char hidden_path[PATH_MAX] = {0, };
+
VALIDATE_OR_GOTO (frame, out);
VALIDATE_OR_GOTO (this, out);
@@ -3439,75 +3524,16 @@ posix_do_readdir (call_frame_t *frame, xlator_t *this,
}
- if (!off) {
- rewinddir (dir);
- } else {
- seekdir (dir, off);
- }
-
- while (filled <= size) {
- in_case = telldir (dir);
-
- if (in_case == -1) {
- op_errno = errno;
- gf_log (this->name, GF_LOG_ERROR,
- "telldir failed on dir=%p: %s",
- dir, strerror (errno));
- goto out;
- }
-
- errno = 0;
- entry = readdir (dir);
-
- if (!entry) {
- if (errno == EBADF) {
- op_errno = errno;
- gf_log (this->name, GF_LOG_WARNING,
- "readdir failed on dir=%p: %s",
- dir, strerror (op_errno));
- goto out;
- }
- break;
- }
-
- if ((!strcmp(real_path, base_path))
- && (!strcmp(entry->d_name, GF_REPLICATE_TRASH_DIR)))
- continue;
-
- if ((!strcmp (real_path, base_path))
- && (!strncmp (GF_HIDDEN_PATH, entry->d_name,
- strlen(GF_HIDDEN_PATH)))) {
- snprintf (hidden_path, PATH_MAX, "%s/%s", real_path,
- entry->d_name);
- ret = lstat (hidden_path, &statbuf);
- if (!ret && S_ISDIR (statbuf.st_mode))
- continue;
- }
- this_size = max (sizeof (gf_dirent_t),
- sizeof (gfs3_dirplist))
- + strlen (entry->d_name) + 1;
-
- if (this_size + filled > size) {
- seekdir (dir, in_case);
- break;
- }
-
- this_entry = gf_dirent_for_name (entry->d_name);
-
- if (!this_entry) {
- gf_log (this->name, GF_LOG_ERROR,
- "could not create gf_dirent for entry %s: (%s)",
- entry->d_name, strerror (errno));
- goto out;
- }
- this_entry->d_off = telldir (dir);
- this_entry->d_ino = entry->d_ino;
-
- list_add_tail (&this_entry->list, &entries.list);
+ LOCK (&fd->lock);
+ {
+ count = __posix_fill_readdir (dir, off, size, &entries,
+ real_path, base_path);
- filled += this_size;
- count ++;
}
+ UNLOCK (&fd->lock);
+
+ /* pick ENOENT to indicate EOF */
+ op_errno = errno;
if (whichop == GF_FOP_READDIRP) {
list_for_each_entry (tmp_entry, &entries.list, list) {
@@ -3519,10 +3545,8 @@ posix_do_readdir (call_frame_t *frame, xlator_t *this,
tmp_entry->d_stat = stbuf;
}
}
+
op_ret = count;
- errno = 0;
- if ((!readdir (dir) && (errno == 0)))
- op_errno = ENOENT;
out:
STACK_UNWIND_STRICT (readdir, frame, op_ret, op_errno, &entries);