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, 90 insertions, 77 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-hooks.c b/xlators/mgmt/glusterd/src/glusterd-hooks.c
index 45a5912a1eb..cb3d38d2358 100644
--- a/xlators/mgmt/glusterd/src/glusterd-hooks.c
+++ b/xlators/mgmt/glusterd/src/glusterd-hooks.c
@@ -323,15 +323,16 @@ glusterd_hooks_run_hooks (char *hooks_path, glusterd_op_t op, dict_t *op_ctx,
 {
         xlator_t        *this                   = NULL;
         glusterd_conf_t *priv                   = NULL;
-        runner_t        runner                  = {0, };
-        struct dirent   *entry                  = NULL;
+        runner_t         runner                 = {0,};
         DIR             *hookdir                = NULL;
+        struct dirent   *entry                  = NULL;
+        struct dirent    scratch[2]             = {{0,},};
         char            *volname                = NULL;
-        char            **lines                 = NULL;
-        int             N                       = 8; /*arbitrary*/
-        int             lineno                  = 0;
-        int             line_count              = 0;
-        int             ret                     = -1;
+        char           **lines                  = NULL;
+        int              N                      = 8; /*arbitrary*/
+        int              lineno                 = 0;
+        int              line_count             = 0;
+        int              ret                    = -1;
 
         this = THIS;
         priv = this->private;
@@ -362,7 +363,7 @@ glusterd_hooks_run_hooks (char *hooks_path, glusterd_op_t op, dict_t *op_ctx,
 
         ret = -1;
         line_count = 0;
-        GF_FOR_EACH_ENTRY_IN_DIR (entry, hookdir);
+        GF_FOR_EACH_ENTRY_IN_DIR (entry, hookdir, scratch);
         while (entry) {
                 if (line_count == N-1) {
                         N *= 2;
@@ -376,7 +377,7 @@ glusterd_hooks_run_hooks (char *hooks_path, glusterd_op_t op, dict_t *op_ctx,
                         line_count++;
                 }
 
-                GF_FOR_EACH_ENTRY_IN_DIR (entry, hookdir);
+                GF_FOR_EACH_ENTRY_IN_DIR (entry, hookdir, scratch);
         }
 
         lines[line_count] = NULL;
diff --git a/xlators/mgmt/glusterd/src/glusterd-quota.c b/xlators/mgmt/glusterd/src/glusterd-quota.c
index 55699cc57a9..0d7113bc1a0 100644
--- a/xlators/mgmt/glusterd/src/glusterd-quota.c
+++ b/xlators/mgmt/glusterd/src/glusterd-quota.c
@@ -392,10 +392,11 @@ void
 glusterd_stop_all_quota_crawl_service (glusterd_conf_t *priv,
                                        glusterd_volinfo_t *volinfo, int type)
 {
-        char                       pid_dir[PATH_MAX]  = {0, };
-        char                       pidfile[PATH_MAX]  = {0,};
-        struct dirent             *entry              = NULL;
         DIR                       *dir                = NULL;
+        struct dirent             *entry              = NULL;
+        struct dirent              scratch[2]         = {{0,},};
+        char                       pid_dir[PATH_MAX]  = {0,};
+        char                       pidfile[PATH_MAX]  = {0,};
 
         GLUSTERD_GET_QUOTA_CRAWL_PIDDIR (pid_dir, volinfo, type);
 
@@ -403,7 +404,7 @@ glusterd_stop_all_quota_crawl_service (glusterd_conf_t *priv,
         if (dir == NULL)
                 return;
 
-        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
         while (entry) {
                 snprintf (pidfile, sizeof (pidfile), "%s/%s",
                           pid_dir, entry->d_name);
@@ -412,7 +413,7 @@ glusterd_stop_all_quota_crawl_service (glusterd_conf_t *priv,
                                               _gf_true);
                 sys_unlink (pidfile);
 
-                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
         }
         sys_closedir (dir);
 }
diff --git a/xlators/mgmt/glusterd/src/glusterd-snapshot-utils.c b/xlators/mgmt/glusterd/src/glusterd-snapshot-utils.c
index 46c8bad1aa9..1765df3d0ef 100644
--- a/xlators/mgmt/glusterd/src/glusterd-snapshot-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-snapshot-utils.c
@@ -3443,12 +3443,13 @@ out:
 int32_t
 glusterd_copy_folder (const char *source, const char *destination)
 {
-        DIR             *dir_ptr                =       NULL;
-        struct dirent   *direntp                =       NULL;
-        int32_t         ret                     =       -1;
-        char            src_path[PATH_MAX]      =       "";
-        char            dest_path[PATH_MAX]     =       "";
-        xlator_t        *this                   =       NULL;
+        int32_t         ret                 = -1;
+        xlator_t       *this                = NULL;
+        DIR            *dir_ptr             = NULL;
+        struct dirent  *entry               = NULL;
+        struct dirent   scratch[2]          = {{0,},};
+        char            src_path[PATH_MAX]  = {0,};
+        char            dest_path[PATH_MAX] = {0,};
 
         this = THIS;
         GF_ASSERT (this);
@@ -3463,17 +3464,22 @@ glusterd_copy_folder (const char *source, const char *destination)
                 goto out;
         }
 
-        while ((direntp = sys_readdir (dir_ptr)) != NULL) {
-                if (strcmp (direntp->d_name, ".") == 0 ||
-                    strcmp (direntp->d_name, "..") == 0)
+        for (;;) {
+                errno = 0;
+                entry = sys_readdir (dir_ptr, scratch);
+                if (!entry || errno != 0)
+                        break;
+
+                if (strcmp (entry->d_name, ".") == 0 ||
+                    strcmp (entry->d_name, "..") == 0)
                         continue;
                 ret = snprintf (src_path, sizeof (src_path), "%s/%s",
-                                source, direntp->d_name);
+                                source, entry->d_name);
                 if (ret < 0)
                         goto out;
 
                 ret = snprintf (dest_path, sizeof (dest_path), "%s/%s",
-                                destination, direntp->d_name);
+                                destination, entry->d_name);
                 if (ret < 0)
                         goto out;
 
@@ -3487,7 +3493,7 @@ glusterd_copy_folder (const char *source, const char *destination)
         }
 out:
         if (dir_ptr)
-                sys_closedir (dir_ptr);
+                (void) sys_closedir (dir_ptr);
 
         return ret;
 }
diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index 8b903ba1e48..deaa0892afe 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.c
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c
@@ -626,14 +626,15 @@ out:
 int32_t
 glusterd_store_remove_bricks (glusterd_volinfo_t *volinfo, char *delete_path)
 {
-        int32_t                 ret = 0;
-        glusterd_brickinfo_t    *tmp = NULL;
-        glusterd_conf_t         *priv = NULL;
-        char                    brickdir [PATH_MAX] = {0,};
-        DIR                     *dir = NULL;
-        struct dirent           *entry = NULL;
-        char                    path[PATH_MAX] = {0,};
-        xlator_t                *this = NULL;
+        int32_t                ret = 0;
+        glusterd_brickinfo_t  *tmp = NULL;
+        glusterd_conf_t       *priv = NULL;
+        xlator_t              *this = NULL;
+        DIR                   *dir = NULL;
+        struct dirent         *entry = NULL;
+        struct dirent          scratch[2] = {{0,},};
+        char                   path[PATH_MAX] = {0,};
+        char                   brickdir[PATH_MAX] = {0,};
 
         this = THIS;
         GF_ASSERT (this);
@@ -654,7 +655,7 @@ glusterd_store_remove_bricks (glusterd_volinfo_t *volinfo, char *delete_path)
 
         dir = sys_opendir (brickdir);
 
-        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
 
         while (entry) {
                 snprintf (path, sizeof (path), "%s/%s",
@@ -664,7 +665,7 @@ glusterd_store_remove_bricks (glusterd_volinfo_t *volinfo, char *delete_path)
                         gf_msg_debug (this->name, 0, "Unable to unlink %s",
                                       path);
                 }
-                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
         }
 
         sys_closedir (dir);
@@ -1772,6 +1773,7 @@ glusterd_store_delete_snap (glusterd_snap_t *snap)
         glusterd_conf_t *priv                  = NULL;
         DIR             *dir                   = NULL;
         struct dirent   *entry                 = NULL;
+        struct dirent    scratch[2]            = {{0,},};
         char             path[PATH_MAX]        = {0,};
         char             delete_path[PATH_MAX] = {0,};
         char             trashdir[PATH_MAX]    = {0,};
@@ -1819,7 +1821,7 @@ glusterd_store_delete_snap (glusterd_snap_t *snap)
                 goto out;
         }
 
-        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
         while (entry) {
                 snprintf (path, PATH_MAX, "%s/%s", delete_path, entry->d_name);
                 ret = sys_stat (path, &st);
@@ -1844,7 +1846,7 @@ glusterd_store_delete_snap (glusterd_snap_t *snap)
                                 entry->d_name);
 stat_failed:
                 memset (path, 0, sizeof(path));
-                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
         }
 
         ret = sys_closedir (dir);
@@ -3111,6 +3113,7 @@ glusterd_store_retrieve_volumes (xlator_t  *this, glusterd_snap_t *snap)
         glusterd_conf_t       *priv             = NULL;
         DIR                   *dir              = NULL;
         struct dirent         *entry            = NULL;
+        struct dirent          scratch[2]       = {{0,},};
         glusterd_volinfo_t    *volinfo          = NULL;
 
         GF_ASSERT (this);
@@ -3133,7 +3136,7 @@ glusterd_store_retrieve_volumes (xlator_t  *this, glusterd_snap_t *snap)
                 goto out;
         }
 
-        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
 
         while (entry) {
                 if (snap && ((!strcmp (entry->d_name, "geo-replication")) ||
@@ -3161,7 +3164,7 @@ glusterd_store_retrieve_volumes (xlator_t  *this, glusterd_snap_t *snap)
 
                 }
 next:
-                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
         }
 
         ret = 0;
@@ -3678,6 +3681,7 @@ glusterd_store_retrieve_snaps (xlator_t  *this)
         glusterd_conf_t       *priv             = NULL;
         DIR                   *dir              = NULL;
         struct dirent         *entry            = NULL;
+        struct dirent          scratch[2]       = {{0,},};
 
         GF_ASSERT (this);
         priv = this->private;
@@ -3700,7 +3704,7 @@ glusterd_store_retrieve_snaps (xlator_t  *this)
                 goto out;
         }
 
-        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
 
         while (entry) {
                 if (strcmp (entry->d_name, GLUSTERD_MISSED_SNAPS_LIST_FILE)) {
@@ -3713,7 +3717,7 @@ glusterd_store_retrieve_snaps (xlator_t  *this)
                                 goto out;
                         }
                 }
-                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
         }
 
         /* Retrieve missed_snaps_list */
@@ -4109,6 +4113,7 @@ glusterd_store_retrieve_peers (xlator_t *this)
         glusterd_conf_t          *priv               = NULL;
         DIR                      *dir                = NULL;
         struct dirent            *entry              = NULL;
+        struct dirent             scratch[2]         = {{0,},};
         char                      path[PATH_MAX]     = {0,};
         glusterd_peerinfo_t      *peerinfo           = NULL;
         gf_store_handle_t        *shandle            = NULL;
@@ -4138,7 +4143,7 @@ glusterd_store_retrieve_peers (xlator_t *this)
                 goto out;
         }
 
-        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+        GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
 
         while (entry) {
                 snprintf (filepath, PATH_MAX, "%s/%s", path, entry->d_name);
@@ -4216,7 +4221,7 @@ glusterd_store_retrieve_peers (xlator_t *this)
 
                 peerinfo->shandle = shandle;
                 peerinfo = NULL;
-                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir);
+                GF_FOR_EACH_ENTRY_IN_DIR (entry, dir, scratch);
         }
 
         args.mode = GD_MODE_ON;
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
index b56e9f26379..6a755486d7d 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -913,58 +913,59 @@ out:
 static void
 volgen_apply_filters (char *orig_volfile)
 {
-        DIR           *filterdir = NULL;
-        struct dirent  entry = {0,};
-        struct dirent *next = NULL;
-        char          *filterpath = NULL;
-        struct stat    statbuf = {0,};
+        DIR           *filterdir  = NULL;
+        struct dirent *entry      = NULL;
+        struct dirent  scratch[2] = {{0,},};
+        struct stat    statbuf    = {0,};
+        char           filterpath[PATH_MAX] = {0,};
 
         filterdir = sys_opendir (FILTERDIR);
-        if (!filterdir) {
+
+        if (!filterdir)
                 return;
-        }
 
-        while ((readdir_r(filterdir,&entry,&next) == 0) && next) {
-                if (!strncmp(entry.d_name,".",sizeof(entry.d_name))) {
-                        continue;
-                }
-                if (!strncmp(entry.d_name,"..",sizeof(entry.d_name))) {
+        for (;;) {
+
+                errno = 0;
+
+                entry = sys_readdir (filterdir, scratch);
+
+                if (!entry || errno != 0)
+                        break;
+
+                if (strcmp (entry->d_name, ".") == 0 ||
+                    strcmp (entry->d_name, "..") == 0)
                         continue;
-                }
                 /*
                  * d_type isn't guaranteed to be present/valid on all systems,
                  * so do an explicit stat instead.
                  */
-                if (gf_asprintf(&filterpath,"%s/%.*s",FILTERDIR,
-                                sizeof(entry.d_name), entry.d_name) == (-1)) {
-                        continue;
-                }
+                (void) snprintf (filterpath, sizeof(filterpath), "%s/%s",
+                                 FILTERDIR, entry->d_name);
+
                 /* Deliberately use stat instead of lstat to allow symlinks. */
-                if (sys_stat(filterpath, &statbuf) == (-1)) {
-                        goto free_fp;
-                }
-                if (!S_ISREG(statbuf.st_mode)) {
-                        goto free_fp;
-                }
+                if (sys_stat (filterpath, &statbuf) == -1)
+                        continue;
+
+                if (!S_ISREG (statbuf.st_mode))
+                        continue;
                 /*
                  * We could check the mode in statbuf directly, or just skip
                  * this entirely and check for EPERM after exec fails, but this
                  * is cleaner.
                  */
-                if (sys_access(filterpath, X_OK) != 0) {
-                        goto free_fp;
-                }
-                if (runcmd(filterpath,orig_volfile,NULL)) {
+                if (sys_access (filterpath, X_OK) != 0)
+                        continue;
+
+                if (runcmd (filterpath, orig_volfile, NULL)) {
                         gf_msg ("glusterd", GF_LOG_ERROR, 0,
                                 GD_MSG_FILTER_RUN_FAILED,
-                                "failed to run filter %.*s",
-                                (int)sizeof(entry.d_name), entry.d_name);
+                                "failed to run filter %s",
+                                entry->d_name);
                 }
-free_fp:
-                GF_FREE(filterpath);
         }
 
-        sys_closedir (filterdir);
+        (void) sys_closedir (filterdir);
 }
 
 static int
@@ -979,7 +980,6 @@ volgen_write_volfile (volgen_graph_t *graph, char *filename)
 
         if (gf_asprintf (&ftmp, "%s.tmp", filename) == -1) {
                 ftmp = NULL;
-
                 goto error;
         }