diff options
| -rw-r--r-- | cli/src/cli-cmd-parser.c | 10 | ||||
| -rw-r--r-- | cli/src/cli-cmd-volume.c | 2 | ||||
| -rw-r--r-- | rpc/rpc-lib/src/protocol-common.h | 2 | ||||
| -rwxr-xr-x | tests/bugs/bug-1085330.t | 80 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-op-sm.h | 1 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-store.c | 44 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-utils.c | 2 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-volgen.c | 3 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 13 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd.h | 19 |
10 files changed, 152 insertions, 24 deletions
diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c index e7f41fa7203..a41c0800ac1 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 83b923e360a..6072fcc5193 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 634fff8f760..02e0d244a4f 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 00000000000..dafba215540 --- /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 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 { \ |