glusterd: fix segfault on volume status detail

If for some reason glusterd_get_brick_root() fails,
it frees the gf_strdup'ed *mount_point in its own error path,
and returns -1.

Unfortunately it already had assigned that pointer value
to the output argument, the caller function
glusterd_add_brick_detail() sees a non-NULL pointer,
and free() again: segfault.

Could be fixed with a one-liner (*mount_point = NULL)
in the error path, but I think glusterd_get_brick_root()
should only assign to the output argument once all checks passed,
so I use a local temporary pointer, which increases the patch a bit.

Change-Id: I3f3035f01e80a5e9bdf2da895e4cf7baa3dfbd2f
BUG: 919352
Signed-off-by: Lars Ellenberg <lars@linbit.com>
Reviewed-on: http://review.gluster.org/4646
Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-on: http://review.gluster.org/4841
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>

1 files changed, 11 insertions, 9 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 4db538b02..83342afb5 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -4193,22 +4193,23 @@ static int
 glusterd_get_brick_root (char *path, char **mount_point)
 {
         char           *ptr            = NULL;
+        char           *mnt_pt         = NULL;
         struct stat     brickstat      = {0};
         struct stat     buf            = {0};
 
         if (!path)
                 goto err;
-        *mount_point = gf_strdup (path);
-        if (!*mount_point)
+        mnt_pt = gf_strdup (path);
+        if (!mnt_pt)
                 goto err;
-        if (stat (*mount_point, &brickstat))
+        if (stat (mnt_pt, &brickstat))
                 goto err;
 
-        while ((ptr = strrchr (*mount_point, '/')) &&
-               ptr != *mount_point) {
+        while ((ptr = strrchr (mnt_pt, '/')) &&
+               ptr != mnt_pt) {
 
                 *ptr = '\0';
-                if (stat (*mount_point, &buf)) {
+                if (stat (mnt_pt, &buf)) {
                         gf_log (THIS->name, GF_LOG_ERROR, "error in "
                                 "stat: %s", strerror (errno));
                         goto err;
@@ -4220,20 +4221,21 @@ glusterd_get_brick_root (char *path, char **mount_point)
                 }
         }
 
-        if (ptr == *mount_point) {
+        if (ptr == mnt_pt) {
                 if (stat ("/", &buf)) {
                         gf_log (THIS->name, GF_LOG_ERROR, "error in "
                                 "stat: %s", strerror (errno));
                         goto err;
                 }
                 if (brickstat.st_dev == buf.st_dev)
-                        strcpy (*mount_point, "/");
+                        strcpy (mnt_pt, "/");
         }
 
+        *mount_point = mnt_pt;
         return 0;
 
  err:
-        GF_FREE (*mount_point);
+        GF_FREE (mnt_pt);
         return -1;
 }