From 561746080b0b7154bfb3bdee20d426cf2ef7db17 Mon Sep 17 00:00:00 2001 From: "Kaleb S. KEITHLEY" Date: Thu, 7 Jul 2016 08:51:08 -0400 Subject: 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 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 Reviewed-on: http://review.gluster.org/14838 Smoke: Gluster Build System Reviewed-by: Niels de Vos CentOS-regression: Gluster Build System NetBSD-regression: NetBSD Build System Reviewed-by: Kotresh HR Reviewed-by: Jeff Darcy --- xlators/mgmt/glusterd/src/glusterd-hooks.c | 19 ++++--- xlators/mgmt/glusterd/src/glusterd-quota.c | 11 ++-- .../mgmt/glusterd/src/glusterd-snapshot-utils.c | 30 ++++++---- xlators/mgmt/glusterd/src/glusterd-store.c | 41 ++++++++------ xlators/mgmt/glusterd/src/glusterd-volgen.c | 66 +++++++++++----------- 5 files changed, 90 insertions(+), 77 deletions(-) (limited to 'xlators/mgmt') 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; } -- cgit