From: Lars Ellenberg <lars@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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
