[Gluster-devel] [PATCH] [release-3.3] glusterd: fix segfault on volume status detail

[Lars Ellenberg]

From: Lars Ellenberg <lars at linbit.com>

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.

Signed-off-by: Lars Ellenberg <lars at linbit.com>
---
 xlators/mgmt/glusterd/src/glusterd-utils.c |   22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 9346e1d..ebfda90 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -3595,22 +3595,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;
@@ -3622,21 +3623,22 @@ 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:
-        if (*mount_point)
-                GF_FREE (*mount_point);
+        if (mnt_pt)
+                GF_FREE (mnt_pt);
         return -1;
 }
 
-- 
1.7.9.5