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>

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;
 }