glusterd : Volname, brickpath & volfpath length validation

While creating a volume and adding a brick validation for _POSIX_PATH_MAX is
done on absolute pathname instead of relative pathname due to which a brickpath
having less than _POSIX_PATH_MAX may also fail the validation if the directory
length is greater than (_POSIX_PATH_MAX -strlen(brickpath/volume name).

Also this fix addresses one cli response message correction which says the
volume file is too long instead of brick path is too long (when brickpath
length validation doesn't fail and vol file length validation fails.)

It is also important to note that with the current design of volfile naming, it
can not be guranteed that volname and brickpath can have max of _POSIX_PATH_MAX
characters.

Change-Id: I1283d1f9dea96ae797620002c8723719f26a866d
BUG: 1085330
Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
Reviewed-on: http://review.gluster.org/7420
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>

6 files changed, 61 insertions, 21 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.h b/xlators/mgmt/glusterd/src/glusterd-op-sm.h
index cb673192d0c..229ee469598 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.h
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.h
@@ -31,7 +31,6 @@
 #include "glusterd.h"
 #include "protocol-common.h"
 
-#define GD_VOLUME_NAME_MAX 256
 #define GD_OP_PROTECTED    (0x02)
 #define GD_OP_UNPROTECTED  (0x04)
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index 0404e110cb5..26de774e3f5 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.c
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c
@@ -134,12 +134,16 @@ glusterd_store_brickinfopath_set (glusterd_volinfo_t *volinfo,
 gf_boolean_t
 glusterd_store_is_valid_brickpath (char *volname, char *brick)
 {
-        char                    brickpath[PATH_MAX] = {0};
         glusterd_brickinfo_t    *brickinfo = NULL;
         glusterd_volinfo_t      *volinfo = NULL;
         int32_t                 ret = 0;
         size_t                  volname_len = strlen (volname);
         xlator_t                *this = NULL;
+        int                     bpath_len = 0;
+        const char              delim[2] = "/";
+        char                    *sub_dir = NULL;
+        char                    *saveptr = NULL;
+        char                    *brickpath_ptr = NULL;
 
         this = THIS;
         GF_ASSERT (this);
@@ -163,10 +167,40 @@ glusterd_store_is_valid_brickpath (char *volname, char *brick)
                 goto out;
         }
         memcpy (volinfo->volname, volname, volname_len+1);
-        glusterd_store_brickinfopath_set (volinfo, brickinfo, brickpath,
-                                                sizeof (brickpath));
 
-        ret = (strlen (brickpath) < _POSIX_PATH_MAX);
+        /* Check whether brickpath is less than PATH_MAX */
+        ret = 1;
+        bpath_len = strlen (brickinfo->path);
+
+        if (brickinfo->path[bpath_len - 1] != '/') {
+                if (strlen (brickinfo->path) >= PATH_MAX) {
+                        ret = 0;
+                        goto out;
+                }
+        } else {
+                /* Path has a trailing "/" which should not be considered in
+                 * length check validation
+                 */
+                if (strlen (brickinfo->path) >= PATH_MAX + 1) {
+                        ret = 0;
+                        goto out;
+                }
+        }
+
+        /* The following validation checks whether each sub directories in the
+         * brick path meets the POSIX max length validation
+         */
+
+        brickpath_ptr = brickinfo->path;
+        sub_dir = strtok_r (brickpath_ptr, delim, &saveptr);
+
+        while (sub_dir != NULL) {
+                if (strlen(sub_dir) >= _POSIX_PATH_MAX) {
+                        ret = 0;
+                        goto out;
+                }
+                sub_dir = strtok_r (NULL, delim, &saveptr);
+        }
 
 out:
         if (brickinfo)
@@ -2564,7 +2598,7 @@ glusterd_store_retrieve_volume (char *volname, glusterd_snap_t *snap)
         if (ret)
                 goto out;
 
-        strncpy (volinfo->volname, volname, GLUSTERD_MAX_VOLUME_NAME);
+        strncpy (volinfo->volname, volname, GD_VOLUME_NAME_MAX);
         volinfo->snapshot = snap;
         if (snap)
                 volinfo->is_snap_volume = _gf_true;
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index b7f81bf83e5..7fd7eaeec5c 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -511,7 +511,7 @@ glusterd_volinfo_new (glusterd_volinfo_t **volinfo)
                 goto out;
         }
 
-        snprintf (new_volinfo->parent_volname, GLUSTERD_MAX_VOLUME_NAME, "N/A");
+        snprintf (new_volinfo->parent_volname, GD_VOLUME_NAME_MAX, "N/A");
 
         new_volinfo->snap_max_hard_limit = GLUSTERD_SNAPS_MAX_HARD_LIMIT;
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
index 82ff8826843..a8aa577be80 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -3479,7 +3479,8 @@ glusterd_is_valid_volfpath (char *volname, char *brick)
         strncpy (volinfo->volname, volname, sizeof (volinfo->volname));
         get_brick_filepath (volfpath, volinfo, brickinfo);
 
-        ret = (strlen (volfpath) < _POSIX_PATH_MAX);
+        ret = ((strlen(volfpath) < PATH_MAX) &&
+                strlen (strrchr(volfpath, '/')) < _POSIX_PATH_MAX);
 
 out:
         if (brickinfo)
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
index 083c7a036ad..8d126c5cc1a 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
@@ -800,14 +800,21 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr)
                 brick= strtok_r (brick_list, " \n", &tmpptr);
                 brick_list = tmpptr;
 
-                if (!glusterd_store_is_valid_brickpath (volname, brick) ||
-                        !glusterd_is_valid_volfpath (volname, brick)) {
+                if (!glusterd_store_is_valid_brickpath (volname, brick)) {
                         snprintf (msg, sizeof (msg), "brick path %s is too "
                                   "long.", brick);
                         ret = -1;
                         goto out;
                 }
 
+                if (!glusterd_is_valid_volfpath (volname, brick)) {
+                        snprintf (msg, sizeof (msg), "Volume file path for "
+                                  "volume %s and brick path %s is too long.",
+                                   volname, brick);
+                        ret = -1;
+                        goto out;
+                }
+
                 ret = glusterd_brickinfo_new_from_brick (brick, &brick_info);
                 if (ret)
                         goto out;
@@ -1522,7 +1529,7 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
                 goto out;
         }
 
-        strncpy (volinfo->volname, volname, GLUSTERD_MAX_VOLUME_NAME);
+        strncpy (volinfo->volname, volname, sizeof(volinfo->volname));
         GF_ASSERT (volinfo->volname);
 
         ret = dict_get_int32 (dict, "type", &volinfo->type);
diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h
index b7c0aeafb1e..2f63d07aac4 100644
--- a/xlators/mgmt/glusterd/src/glusterd.h
+++ b/xlators/mgmt/glusterd/src/glusterd.h
@@ -39,7 +39,6 @@
 #include "syncop.h"
 #include "store.h"
 
-#define GLUSTERD_MAX_VOLUME_NAME        1000
 #define GLUSTERD_TR_LOG_SIZE            50
 #define GLUSTERD_NAME                   "glusterd"
 #define GLUSTERD_SOCKET_LISTEN_BACKLOG  128
@@ -297,17 +296,17 @@ typedef struct glusterd_replace_brick_ glusterd_replace_brick_t;
 
 struct glusterd_volinfo_ {
         gf_lock_t                 lock;
-        char                      volname[GLUSTERD_MAX_VOLUME_NAME];
         gf_boolean_t              is_snap_volume;
         glusterd_snap_t          *snapshot;
         uuid_t                    restored_from_snap;
-        char                      parent_volname[GLUSTERD_MAX_VOLUME_NAME];
+        char                      parent_volname[GD_VOLUME_NAME_MAX];
                                          /* In case of a snap volume
                                             i.e (is_snap_volume == TRUE) this
                                             field will contain the name of
                                             the volume which is snapped. In
                                             case of a non-snap volume, this
                                             field will be initialized as N/A */
+        char                      volname[GD_VOLUME_NAME_MAX];
         int                       type;
         int                       brick_count;
         uint64_t                  snap_count;
@@ -521,13 +520,13 @@ typedef ssize_t (*gd_serialize_t) (struct iovec outmsg, void *args);
         snprintf (abspath, sizeof (abspath)-1,                          \
                   DEFAULT_VAR_RUN_DIRECTORY"/%s%s", volname, path);
 
-#define GLUSTERD_REMOVE_SLASH_FROM_PATH(path,string) do {               \
-                int i = 0;                                              \
-                for (i = 1; i < strlen (path); i++) {                   \
-                        string[i-1] = path[i];                          \
-                        if (string[i-1] == '/')                         \
-                                string[i-1] = '-';                      \
-                }                                                       \
+#define GLUSTERD_REMOVE_SLASH_FROM_PATH(path,string) do {                  \
+                int i = 0;                                                 \
+                for (i = 1; i < strlen (path); i++) {                      \
+                        string[i-1] = path[i];                             \
+                        if (string[i-1] == '/' && (i != strlen(path) - 1)) \
+                                string[i-1] = '-';                         \
+                }                                                          \
         } while (0)
 
 #define GLUSTERD_GET_BRICK_PIDFILE(pidfile,volinfo,brickinfo, priv) do {      \