diff options
author | Kaleb S. KEITHLEY <kkeithle@redhat.com> | 2016-07-07 08:51:08 -0400 |
---|---|---|
committer | Jeff Darcy <jdarcy@redhat.com> | 2016-07-18 04:59:42 -0700 |
commit | 561746080b0b7154bfb3bdee20d426cf2ef7db17 (patch) | |
tree | 0dd0db913055925d7843d85c8066a7c0018a290a /xlators/features | |
parent | 73b9ede7e115fab245b0f59d18e4d6cc4d297cec (diff) |
core: use readdir(3) with glibc, and associated cleanup
Starting with glibc-2.23 (i.e. what's in Fedora 25), readdir_r(3)
is marked as deprecated. Specifically the function decl in <dirent.h>
has the deprecated attribute, and now warnings are thrown during the
compile on Fedora 25 builds.
The readdir(_r)(3) man page (on Fedora 25 at least) and World+Dog say
that glibc's readdir(3) is, and always has been, MT-SAFE as long as
only one thread is accessing the directory object returned by opendir().
World+Dog also says there is a potential buffer overflow in readdir_r().
World+Dog suggests that it is preferable to simply use readdir(). There's
an implication that eventually readdir_r(3) will be removed from glibc.
POSIX has, apparently deprecated it in the standard, or even removed it
entirely.
Over and above that, our source near the various uses of readdir(_r)(3)
has a few unsafe uses of strcpy()+strcat().
(AFAIK nobody has looked at the readdir(3) implemenation in *BSD to see
if the same is true on those platforms, and we can't be sure of MacOS
even though we know it's based on *BSD.)
Change-Id: I5481f18ba1eebe7ee177895eecc9a80a71b60568
BUG: 1356998
Signed-off-by: Kaleb S. KEITHLEY <kkeithle@redhat.com>
Reviewed-on: http://review.gluster.org/14838
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Niels de Vos <ndevos@redhat.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-by: Kotresh HR <khiremat@redhat.com>
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
Diffstat (limited to 'xlators/features')
-rw-r--r-- | xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c | 26 | ||||
-rw-r--r-- | xlators/features/changelog/lib/src/gf-changelog-api.c | 42 | ||||
-rw-r--r-- | xlators/features/changelog/lib/src/gf-history-changelog.c | 59 | ||||
-rw-r--r-- | xlators/features/changelog/src/changelog-mem-types.h | 11 | ||||
-rw-r--r-- | xlators/features/index/src/index.c | 129 |
5 files changed, 137 insertions, 130 deletions
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c b/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c index 7bd270d1513..09f556db10b 100644 --- a/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c +++ b/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c @@ -465,15 +465,15 @@ static int br_stub_fill_readdir (fd_t *fd, br_stub_fd_t *fctx, DIR *dir, off_t off, size_t size, gf_dirent_t *entries) { - off_t in_case = -1; - off_t last_off = 0; - size_t filled = 0; - int count = 0; - char entrybuf[sizeof(struct dirent) + 256 + 8]; - struct dirent *entry = NULL; - int32_t this_size = -1; - gf_dirent_t *this_entry = NULL; - xlator_t *this = NULL; + off_t in_case = -1; + off_t last_off = 0; + size_t filled = 0; + int count = 0; + int32_t this_size = -1; + gf_dirent_t *this_entry = NULL; + xlator_t *this = NULL; + struct dirent *entry = NULL; + struct dirent scratch[2] = {{0,},}; this = THIS; if (!off) { @@ -507,10 +507,8 @@ br_stub_fill_readdir (fd_t *fd, br_stub_fd_t *fctx, DIR *dir, off_t off, } errno = 0; - entry = NULL; - readdir_r (dir, (struct dirent *)entrybuf, &entry); - - if (!entry) { + entry = sys_readdir (dir, scratch); + if (!entry || errno != 0) { if (errno == EBADF) { gf_msg (THIS->name, GF_LOG_WARNING, 0, BRS_MSG_BAD_OBJECT_DIR_READ_FAIL, @@ -579,7 +577,7 @@ br_stub_fill_readdir (fd_t *fd, br_stub_fd_t *fctx, DIR *dir, off_t off, count++; } - if ((!sys_readdir (dir) && (errno == 0))) { + if ((!sys_readdir (dir, scratch) && (errno == 0))) { /* Indicate EOF */ errno = ENOENT; /* Remember EOF offset for later detection */ diff --git a/xlators/features/changelog/lib/src/gf-changelog-api.c b/xlators/features/changelog/lib/src/gf-changelog-api.c index f41b505a749..d2a28bc6d52 100644 --- a/xlators/features/changelog/lib/src/gf-changelog-api.c +++ b/xlators/features/changelog/lib/src/gf-changelog-api.c @@ -152,16 +152,16 @@ out: ssize_t gf_changelog_scan () { - int ret = 0; - int tracker_fd = 0; - size_t len = 0; - size_t off = 0; - xlator_t *this = NULL; - size_t nr_entries = 0; + int ret = 0; + int tracker_fd = 0; + size_t len = 0; + size_t off = 0; + xlator_t *this = NULL; + size_t nr_entries = 0; gf_changelog_journal_t *jnl = NULL; - struct dirent *entryp = NULL; - struct dirent *result = NULL; - char buffer[PATH_MAX] = {0,}; + struct dirent *entry = NULL; + struct dirent scratch[2] = {{0,},}; + char buffer[PATH_MAX] = {0,}; this = THIS; if (!this) @@ -183,19 +183,17 @@ gf_changelog_scan () len = offsetof(struct dirent, d_name) + pathconf(jnl->jnl_processing_dir, _PC_NAME_MAX) + 1; - entryp = GF_CALLOC (1, len, - gf_changelog_mt_libgfchangelog_dirent_t); - if (!entryp) - goto out; rewinddir (jnl->jnl_dir); - while (1) { - ret = readdir_r (jnl->jnl_dir, entryp, &result); - if (ret || !result) + + for (;;) { + errno = 0; + entry = sys_readdir (jnl->jnl_dir, scratch); + if (!entry || errno != 0) break; - if (!strcmp (basename (entryp->d_name), ".") - || !strcmp (basename (entryp->d_name), "..")) + if (!strcmp (basename (entry->d_name), ".") + || !strcmp (basename (entry->d_name), "..")) continue; nr_entries++; @@ -203,8 +201,8 @@ gf_changelog_scan () GF_CHANGELOG_FILL_BUFFER (jnl->jnl_processing_dir, buffer, off, strlen (jnl->jnl_processing_dir)); - GF_CHANGELOG_FILL_BUFFER (entryp->d_name, buffer, - off, strlen (entryp->d_name)); + GF_CHANGELOG_FILL_BUFFER (entry->d_name, buffer, + off, strlen (entry->d_name)); GF_CHANGELOG_FILL_BUFFER ("\n", buffer, off, 1); if (gf_changelog_write (tracker_fd, buffer, off) != off) { @@ -217,9 +215,7 @@ gf_changelog_scan () off = 0; } - GF_FREE (entryp); - - if (!result) { + if (!entry) { if (gf_lseek (tracker_fd, 0, SEEK_SET) != -1) return nr_entries; } diff --git a/xlators/features/changelog/lib/src/gf-history-changelog.c b/xlators/features/changelog/lib/src/gf-history-changelog.c index 389b65f3b16..5ed50390a7c 100644 --- a/xlators/features/changelog/lib/src/gf-history-changelog.c +++ b/xlators/features/changelog/lib/src/gf-history-changelog.c @@ -222,8 +222,8 @@ gf_history_changelog_scan () size_t nr_entries = 0; gf_changelog_journal_t *jnl = NULL; gf_changelog_journal_t *hist_jnl = NULL; - struct dirent *entryp = NULL; - struct dirent *result = NULL; + struct dirent *entry = NULL; + struct dirent scratch[2] = {{0,},}; char buffer[PATH_MAX] = {0,}; static int is_last_scan; @@ -260,19 +260,17 @@ gf_history_changelog_scan () len = offsetof (struct dirent, d_name) + pathconf (hist_jnl->jnl_processing_dir, _PC_NAME_MAX) + 1; - entryp = GF_CALLOC (1, len, - gf_changelog_mt_libgfchangelog_dirent_t); - if (!entryp) - goto out; rewinddir (hist_jnl->jnl_dir); - while (1) { - ret = readdir_r (hist_jnl->jnl_dir, entryp, &result); - if (ret || !result) + + for (;;) { + errno = 0; + entry = sys_readdir (hist_jnl->jnl_dir, scratch); + if (!entry || errno != 0) break; - if ( !strcmp (basename (entryp->d_name), ".") - || !strcmp (basename (entryp->d_name), "..") ) + if (strcmp (basename (entry->d_name), ".") == 0 || + strcmp (basename (entry->d_name), "..") == 0) continue; nr_entries++; @@ -280,8 +278,8 @@ gf_history_changelog_scan () GF_CHANGELOG_FILL_BUFFER (hist_jnl->jnl_processing_dir, buffer, off, strlen (hist_jnl->jnl_processing_dir)); - GF_CHANGELOG_FILL_BUFFER (entryp->d_name, buffer, - off, strlen (entryp->d_name)); + GF_CHANGELOG_FILL_BUFFER (entry->d_name, buffer, + off, strlen (entry->d_name)); GF_CHANGELOG_FILL_BUFFER ("\n", buffer, off, 1); if (gf_changelog_write (tracker_fd, buffer, off) != off) { @@ -294,13 +292,11 @@ gf_history_changelog_scan () off = 0; } - GF_FREE (entryp); - gf_msg_debug (this->name, 0, "hist_done %d, is_last_scan: %d", hist_jnl->hist_done, is_last_scan); - if (!result) { + if (!entry) { if (gf_lseek (tracker_fd, 0, SEEK_SET) != -1) { if (nr_entries > 0) return nr_entries; @@ -678,7 +674,7 @@ gf_history_consume (void * data) out: if (fd != -1) - sys_close (fd); + (void) sys_close (fd); GF_FREE (hist_data); return NULL; } @@ -794,12 +790,13 @@ gf_history_changelog (char* changelog_dir, unsigned long start, unsigned long to = 0; unsigned long from = 0; unsigned long total_changelog = 0; - xlator_t *this = NULL; - gf_changelog_journal_t *jnl = NULL; - gf_changelog_journal_t *hist_jnl = NULL; - gf_changelog_history_data_t *hist_data = NULL; - DIR *dirp = NULL; - struct dirent *dp = NULL; + xlator_t *this = NULL; + gf_changelog_journal_t *jnl = NULL; + gf_changelog_journal_t *hist_jnl = NULL; + gf_changelog_history_data_t *hist_data = NULL; + DIR *dirp = NULL; + struct dirent *entry = NULL; + struct dirent scratch[2] = {{0,},}; pthread_t consume_th = 0; char htime_dir[PATH_MAX] = {0,}; char buffer[PATH_MAX] = {0,}; @@ -851,8 +848,16 @@ gf_history_changelog (char* changelog_dir, unsigned long start, goto out; } - while ((dp = sys_readdir (dirp)) != NULL) { - ret = gf_changelog_extract_min_max (dp->d_name, htime_dir, + for (;;) { + + errno = 0; + + entry = sys_readdir (dirp, scratch); + + if (!entry || errno != 0) + break; + + ret = gf_changelog_extract_min_max (entry->d_name, htime_dir, &fd, &total_changelog, &min_ts, &max_ts); if (ret) { @@ -968,11 +973,11 @@ gf_history_changelog (char* changelog_dir, unsigned long start, out: if (dirp != NULL) - sys_closedir (dirp); + (void) sys_closedir (dirp); if (ret < 0) { if (fd != -1) - sys_close (fd); + (void) sys_close (fd); GF_FREE (hist_data); (void) pthread_attr_destroy (&attr); diff --git a/xlators/features/changelog/src/changelog-mem-types.h b/xlators/features/changelog/src/changelog-mem-types.h index 1618f722f6c..33fea31b979 100644 --- a/xlators/features/changelog/src/changelog-mem-types.h +++ b/xlators/features/changelog/src/changelog-mem-types.h @@ -23,12 +23,11 @@ enum gf_changelog_mem_types { gf_changelog_mt_libgfchangelog_t = gf_common_mt_end + 7, gf_changelog_mt_libgfchangelog_entry_t = gf_common_mt_end + 8, gf_changelog_mt_libgfchangelog_rl_t = gf_common_mt_end + 9, - gf_changelog_mt_libgfchangelog_dirent_t = gf_common_mt_end + 10, - gf_changelog_mt_changelog_buffer_t = gf_common_mt_end + 11, - gf_changelog_mt_history_data_t = gf_common_mt_end + 12, - gf_changelog_mt_libgfchangelog_call_pool_t = gf_common_mt_end + 13, - gf_changelog_mt_libgfchangelog_event_t = gf_common_mt_end + 14, - gf_changelog_mt_ev_dispatcher_t = gf_common_mt_end + 15, + gf_changelog_mt_changelog_buffer_t = gf_common_mt_end + 10, + gf_changelog_mt_history_data_t = gf_common_mt_end + 11, + gf_changelog_mt_libgfchangelog_call_pool_t = gf_common_mt_end + 12, + gf_changelog_mt_libgfchangelog_event_t = gf_common_mt_end + 13, + gf_changelog_mt_ev_dispatcher_t = gf_common_mt_end + 14, gf_changelog_mt_end }; diff --git a/xlators/features/index/src/index.c b/xlators/features/index/src/index.c index f25697ebebd..75809e36e4c 100644 --- a/xlators/features/index/src/index.c +++ b/xlators/features/index/src/index.c @@ -437,15 +437,15 @@ static int index_fill_readdir (fd_t *fd, index_fd_ctx_t *fctx, DIR *dir, off_t off, size_t size, gf_dirent_t *entries) { - off_t in_case = -1; - off_t last_off = 0; - size_t filled = 0; - int count = 0; - char entrybuf[sizeof(struct dirent) + 256 + 8]; - struct dirent *entry = NULL; - int32_t this_size = -1; - gf_dirent_t *this_entry = NULL; - xlator_t *this = NULL; + off_t in_case = -1; + off_t last_off = 0; + size_t filled = 0; + int count = 0; + struct dirent *entry = NULL; + struct dirent scratch[2] = {{0,},}; + int32_t this_size = -1; + gf_dirent_t *this_entry = NULL; + xlator_t *this = NULL; this = THIS; if (!off) { @@ -477,10 +477,8 @@ index_fill_readdir (fd_t *fd, index_fd_ctx_t *fctx, DIR *dir, off_t off, } errno = 0; - entry = NULL; - readdir_r (dir, (struct dirent *)entrybuf, &entry); - - if (!entry) { + entry = sys_readdir (dir, scratch); + if (!entry || errno != 0) { if (errno == EBADF) { gf_msg (THIS->name, GF_LOG_WARNING, errno, INDEX_MSG_INDEX_READDIR_FAILED, @@ -550,7 +548,9 @@ index_fill_readdir (fd_t *fd, index_fd_ctx_t *fctx, DIR *dir, off_t off, count ++; } - if ((!sys_readdir (dir) && (errno == 0))) { + errno = 0; + + if ((!sys_readdir (dir, scratch) && (errno == 0))) { /* Indicate EOF */ errno = ENOENT; /* Remember EOF offset for later detection */ @@ -1107,7 +1107,7 @@ __index_fd_ctx_get (fd_t *fd, xlator_t *this, index_fd_ctx_t **ctx) ret = __fd_ctx_set (fd, this, (uint64_t)(long)fctx); if (ret) { - sys_closedir (fctx->dir); + (void) sys_closedir (fctx->dir); GF_FREE (fctx); fctx = NULL; ret = -EINVAL; @@ -1374,12 +1374,12 @@ out: uint64_t index_entry_count (xlator_t *this, char *subdir) { - index_priv_t *priv = NULL; - char index_dir[PATH_MAX]; - DIR *dirp = NULL; - uint64_t count = 0; - struct dirent buf; - struct dirent *entry = NULL; + uint64_t count = 0; + index_priv_t *priv = NULL; + DIR *dirp = NULL; + struct dirent *entry = NULL; + struct dirent scratch[2] = {{0,},}; + char index_dir[PATH_MAX] = {0,}; priv = this->private; @@ -1390,17 +1390,23 @@ index_entry_count (xlator_t *this, char *subdir) if (!dirp) return 0; - while (readdir_r (dirp, &buf, &entry) == 0) { - if (!entry) + for (;;) { + errno = 0; + entry = sys_readdir (dirp, scratch); + if (!entry || errno != 0) break; - if (!strcmp (entry->d_name, ".") || - !strcmp (entry->d_name, "..")) + + if (strcmp (entry->d_name, ".") == 0 || + strcmp (entry->d_name, "..") == 0) continue; + if (!strncmp (entry->d_name, subdir, strlen (subdir))) continue; + count++; } - sys_closedir (dirp); + + (void) sys_closedir (dirp); return count; } @@ -1909,53 +1915,56 @@ out: int64_t index_fetch_link_count (xlator_t *this, index_xattrop_type_t type) { - char index_dir[PATH_MAX] = {0}; - char index_path[PATH_MAX] = {0}; - index_priv_t *priv = this->private; - char *subdir = NULL; - DIR *dirp = NULL; - struct dirent *entry = NULL; - struct stat lstatbuf = {0}; - int ret = -1; - int64_t count = -1; - struct dirent buf; + index_priv_t *priv = this->private; + char *subdir = NULL; + struct stat lstatbuf = {0,}; + int ret = -1; + int64_t count = -1; + DIR *dirp = NULL; + struct dirent *entry = NULL; + struct dirent scratch[2] = {{0,},}; + char index_dir[PATH_MAX] = {0,}; + char index_path[PATH_MAX] = {0,}; subdir = index_get_subdir_from_type (type); - make_index_dir_path (priv->index_basepath, subdir, - index_dir, sizeof (index_dir)); + make_index_dir_path (priv->index_basepath, subdir, + index_dir, sizeof (index_dir)); - dirp = sys_opendir (index_dir); - if (!dirp) + dirp = sys_opendir (index_dir); + if (!dirp) goto out; - while (readdir_r (dirp, &buf, &entry) == 0) { - if (!entry) { + for (;;) { + errno = 0; + entry = sys_readdir (dirp, scratch); + if (!entry || errno != 0) { if (count == -1) count = 0; goto out; - } else if (!strcmp (entry->d_name, ".") || - !strcmp (entry->d_name, "..")) { - continue; + } + + if (strcmp (entry->d_name, ".") == 0 || + strcmp (entry->d_name, "..") == 0) + continue; + + make_file_path (priv->index_basepath, subdir, + entry->d_name, index_path, sizeof(index_path)); + + ret = sys_lstat (index_path, &lstatbuf); + if (ret < 0) { + count = -2; + continue; } else { - make_file_path (priv->index_basepath, subdir, - entry->d_name, index_path, - sizeof (index_path)); - ret = sys_lstat (index_path, &lstatbuf); - if (ret < 0) { - count = -2; + count = lstatbuf.st_nlink - 1; + if (count == 0) continue; - } else { - count = lstatbuf.st_nlink - 1; - if (count == 0) - continue; - else - break; - } + else + break; } - } + } out: if (dirp) - sys_closedir (dirp); + (void) sys_closedir (dirp); return count; } |