summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--cli/src/cli-cmd-parser.c10
-rw-r--r--cli/src/cli-cmd-volume.c2
-rw-r--r--rpc/rpc-lib/src/protocol-common.h2
-rwxr-xr-xtests/bugs/bug-1085330.t80
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-op-sm.h1
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-store.c44
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-utils.c2
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-volgen.c3
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-volume-ops.c13
-rw-r--r--xlators/mgmt/glusterd/src/glusterd.h19
10 files changed, 152 insertions, 24 deletions
diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c
index e7f41fa..a41c080 100644
--- a/cli/src/cli-cmd-parser.c
+++ b/cli/src/cli-cmd-parser.c
@@ -236,8 +236,11 @@ cli_cmd_volume_create_parse (const char **words, int wordcount, dict_t **options
if (strchr (volname, '/'))
goto out;
- if (strlen (volname) > 512)
+ if (strlen (volname) > GD_VOLUME_NAME_MAX) {
+ cli_err("Volume name exceeds %d characters.",
+ GD_VOLUME_NAME_MAX);
goto out;
+ }
for (i = 0; i < strlen (volname); i++)
if (!isalnum (volname[i]) && (volname[i] != '_') &&
@@ -561,8 +564,11 @@ cli_cmd_quota_parse (const char **words, int wordcount, dict_t **options)
if (strchr (volname, '/'))
goto out;
- if (strlen (volname) > 512)
+ if (strlen (volname) > GD_VOLUME_NAME_MAX) {
+ cli_err("Volname can not exceed %d characters.",
+ GD_VOLUME_NAME_MAX);
goto out;
+ }
for (i = 0; i < strlen (volname); i++)
if (!isalnum (volname[i]) && (volname[i] != '_') &&
diff --git a/cli/src/cli-cmd-volume.c b/cli/src/cli-cmd-volume.c
index 83b923e..6072fcc 100644
--- a/cli/src/cli-cmd-volume.c
+++ b/cli/src/cli-cmd-volume.c
@@ -65,7 +65,7 @@ cli_cmd_volume_info_cbk (struct cli_state *state, struct cli_cmd_word *word,
} else if (wordcount == 3) {
ctx.flags = GF_CLI_GET_VOLUME;
ctx.volname = (char *)words[2];
- if (strlen (ctx.volname) > 1024) {
+ if (strlen (ctx.volname) > GD_VOLUME_NAME_MAX) {
cli_out ("Invalid volume name");
goto out;
}
diff --git a/rpc/rpc-lib/src/protocol-common.h b/rpc/rpc-lib/src/protocol-common.h
index 634fff8..02e0d24 100644
--- a/rpc/rpc-lib/src/protocol-common.h
+++ b/rpc/rpc-lib/src/protocol-common.h
@@ -292,4 +292,6 @@ typedef struct gf_gsync_detailed_status_ gf_gsync_status_t;
#define GD_MGMT_HNDSK_PROGRAM 1239873 /* Completely random */
#define GD_MGMT_HNDSK_VERSION 1
+#define GD_VOLUME_NAME_MAX 256 /* Maximum size of volume name */
+
#endif /* !_PROTOCOL_COMMON_H */
diff --git a/tests/bugs/bug-1085330.t b/tests/bugs/bug-1085330.t
new file mode 100755
index 0000000..dafba21
--- /dev/null
+++ b/tests/bugs/bug-1085330.t
@@ -0,0 +1,80 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+
+STR="1234567890"
+volname="Vol"
+
+cleanup;
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume info;
+
+
+# Construct volname string such that its more than 256 characters
+for i in {1..30}
+do
+ volname+=$STR
+done
+# Now $volname is more than 256 chars
+
+TEST ! $CLI volume create $volname $H0:$B0/${volname}{1,2};
+
+TEST $CLI volume info;
+
+# Construct brick string such that its more than 256 characters
+volname="Vol1234"
+brick="brick"
+for i in {1..30}
+do
+ brick+=$STR
+done
+# Now $brick1 is more than 256 chars
+
+TEST ! $CLI volume create $volname $H0:$B0/$brick;
+
+TEST $CLI volume info;
+
+# Now try to create a volume with couple of bricks (strlen(volname) = 128 &
+# strlen(brick1) = 128
+# Command should still fail as strlen(volp path) > 256
+
+volname="Volume-0"
+brick="brick-00"
+STR="12345678"
+
+for i in {1..15}
+do
+ volname+=$STR
+ brick+=$STR
+done
+TEST ! $CLI volume create $volname $H0:$B0/$brick;
+
+TEST $CLI volume info;
+
+# test case with brick path as 255 and a trailing "/"
+brick=""
+STR1="12345678"
+volname="vol"
+
+for i in {1..31}
+do
+ brick+=$STR1
+done
+brick+="123456/"
+
+echo $brick | wc -c
+# Now $brick is exactly 255 chars, but at end a trailing space
+# This will still fail as volfpath exceeds more than _POSIX_MAX chars
+
+TEST ! $CLI volume create $volname $H0:$B0/$brick;
+
+TEST $CLI volume info;
+
+# Positive test case
+TEST $CLI volume create $V0 $H0:$B0/${V0}{1,2};
+
+TEST $CLI volume info;
+
+cleanup;
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.h b/xlators/mgmt/glusterd/src/glusterd-op-sm.h
index cb67319..229ee46 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 0404e11..26de774 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 b7f81bf..7fd7eae 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 82ff882..a8aa577 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 083c7a0..8d126c5 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 b7c0aea..2f63d07 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 { \