Glusterd : glusterd process generates core due to NULL store handle

Problem : glusterd crashed as backtrace revealed that store handle was
set to NULL.

Solution : In glusterd_store_global_info() function out block the
handle is dereferenced with out any NULL check which caused this
segmentation fault. A NULL check is introduced to avoid this. While
testing this fix, another issue was noticed where GF_ASSERT macro again
does not gurantee the NULL dereference check and hence this macro call
has been replaced by GF_VALIDATE_OR_GOTO macro call in places where
there is a danger of macro getting crashed due to NULL dereference
check.

Change-Id: Ic301aa45ce4bbdc2da751d2386439df7bb24c016
BUG: 1040844
Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
Reviewed-on: http://review.gluster.org/6619
Reviewed-by: Niels de Vos <ndevos@redhat.com>
Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

2 files changed, 15 insertions, 13 deletions

diff --git a/libglusterfs/src/store.c b/libglusterfs/src/store.c
index 48c79ee0289..5af23592b84 100644
--- a/libglusterfs/src/store.c
+++ b/libglusterfs/src/store.c
@@ -62,8 +62,8 @@ gf_store_mkstemp (gf_store_handle_t *shandle)
         int     fd = -1;
         char    tmppath[PATH_MAX] = {0,};
 
-        GF_ASSERT (shandle);
-        GF_ASSERT (shandle->path);
+        GF_VALIDATE_OR_GOTO ("store", shandle, out);
+        GF_VALIDATE_OR_GOTO ("store", shandle->path, out);
 
         snprintf (tmppath, sizeof (tmppath), "%s.tmp", shandle->path);
         fd = open (tmppath, O_RDWR | O_CREAT | O_TRUNC | O_SYNC, 0600);
@@ -71,7 +71,7 @@ gf_store_mkstemp (gf_store_handle_t *shandle)
                 gf_log ("", GF_LOG_ERROR, "Failed to open %s, error: %s",
                         tmppath, strerror (errno));
         }
-
+out:
         return fd;
 }
 
@@ -127,8 +127,8 @@ gf_store_rename_tmppath (gf_store_handle_t *shandle)
         int32_t         ret = -1;
         char            tmppath[PATH_MAX] = {0,};
 
-        GF_ASSERT (shandle);
-        GF_ASSERT (shandle->path);
+        GF_VALIDATE_OR_GOTO ("store", shandle, out);
+        GF_VALIDATE_OR_GOTO ("store", shandle->path, out);
 
         snprintf (tmppath, sizeof (tmppath), "%s.tmp", shandle->path);
         ret = rename (tmppath, shandle->path);
@@ -149,8 +149,8 @@ gf_store_unlink_tmppath (gf_store_handle_t *shandle)
         int32_t         ret = -1;
         char            tmppath[PATH_MAX] = {0,};
 
-        GF_ASSERT (shandle);
-        GF_ASSERT (shandle->path);
+        GF_VALIDATE_OR_GOTO ("store", shandle, out);
+        GF_VALIDATE_OR_GOTO ("store", shandle->path, out);
 
         snprintf (tmppath, sizeof (tmppath), "%s.tmp", shandle->path);
         ret = unlink (tmppath);
@@ -160,7 +160,7 @@ gf_store_unlink_tmppath (gf_store_handle_t *shandle)
         } else {
                 ret = 0;
         }
-
+out:
         return ret;
 }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index 3a4b090096b..d0ad7dcdb8d 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.c
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c
@@ -1329,12 +1329,14 @@ glusterd_store_global_info (xlator_t *this)
 
         ret = gf_store_rename_tmppath (handle);
 out:
-        if (ret && (handle->fd > 0))
-                gf_store_unlink_tmppath (handle);
+        if (handle) {
+                if (ret && (handle->fd > 0))
+                        gf_store_unlink_tmppath (handle);
 
-        if (handle->fd > 0) {
-                close (handle->fd);
-                handle->fd = 0;
+                if (handle->fd > 0) {
+                        close (handle->fd);
+                        handle->fd = 0;
+                }
         }
 
         if (uuid_str)