From 7c25eb1ba2b1db3009a0e88b3ecf229134f8ac92 Mon Sep 17 00:00:00 2001 From: Bobi Jam Date: Thu, 12 Feb 2015 11:58:26 +0800 Subject: [PATCH] LU-6234 util: check fsname and pool name The pool name length check should not include its fsname. This patch get rid of the possibly added fsname in the pool name argument. Check lustre filesystem name (fsname) in mkfs.lustre, it can only contains alphabetic, digital char and '_'/'-'. Check pool name in lctl/lfs, its character also abides by the constraint of the fsname. Add test cases to check the pool names for length, with and without a filesystem name prefix, and with invalid names. Also fix pool_add in the test-framework so that it greps for the poolname plus the filesystem name, so it doesn't match more lines than necessary. Signed-off-by: Bobi Jam Signed-off-by: frank zago Change-Id: I4f542e1bc7b3ea97b35ec60e51aa971e73da91eb Reviewed-on: http://review.whamcloud.com/13742 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin --- lustre/include/lustre_param.h | 74 +++++++++++++++++++++++++++++++++ lustre/tests/ost-pools.sh | 89 ++++++++++++++++++++++++++++++++++++++++ lustre/tests/test-framework.sh | 2 +- lustre/utils/lfs.c | 41 ++++++++++++++++--- lustre/utils/mkfs_lustre.c | 24 ++++++----- lustre/utils/obd.c | 93 ++++++++++++++++++++++++++---------------- 6 files changed, 270 insertions(+), 53 deletions(-) diff --git a/lustre/include/lustre_param.h b/lustre/include/lustre_param.h index 9ccd63f..f41accf 100644 --- a/lustre/include/lustre_param.h +++ b/lustre/include/lustre_param.h @@ -123,4 +123,78 @@ int do_lcfg(char *cfgname, lnet_nid_t nid, int cmd, /** @} param */ +#define LUSTRE_MAXFSNAME 8 + +/** + * Check whether the name is valid. + * + * \param name [in] the name to be checked + * \param minlen [in] the minimum length of the name + * \param maxlen [in] the maximum length of the name + * + * \retval 0 the name is valid + * \retval >0 the invalid character in the name + * \retval -1 the name is too short + * \retval -2 the name is too long + */ +static inline int lustre_is_name_valid(const char *name, const int minlen, + const int maxlen) +{ + const char *tmp; + size_t len; + + len = strlen(name); + + if (len < minlen) + return -1; + + if (len > maxlen) + return -2; + + for (tmp = name; *tmp != '\0'; ++tmp) { + if (isalnum(*tmp) || *tmp == '_' || *tmp == '-') + continue; + else + break; + } + + return *tmp == '\0' ? 0 : *tmp; +} + +/** + * Check whether the fsname is valid. + * + * \param fsname [in] the fsname to be checked + * \param minlen [in] the minimum length of the fsname + * \param maxlen [in] the maximum length of the fsname + * + * \retval 0 the fsname is valid + * \retval >0 the invalid character in the fsname + * \retval -1 the fsname is too short + * \retval -2 the fsname is too long + */ +static inline int lustre_is_fsname_valid(const char *fsname, const int minlen, + const int maxlen) +{ + return lustre_is_name_valid(fsname, minlen, maxlen); +} + +/** + * Check whether the poolname is valid. + * + * \param poolname [in] the poolname to be checked + * \param minlen [in] the minimum length of the poolname + * \param maxlen [in] the maximum length of the poolname + * + * \retval 0 the poolname is valid + * \retval >0 the invalid character in the poolname + * \retval -1 the poolname is too short + * \retval -2 the poolname is too long + */ +static inline int lustre_is_poolname_valid(const char *poolname, + const int minlen, const int maxlen) +{ + return lustre_is_name_valid(poolname, minlen, maxlen); +} + #endif /* _LUSTRE_PARAM_H */ diff --git a/lustre/tests/ost-pools.sh b/lustre/tests/ost-pools.sh index 865f62f..a53fa90 100644 --- a/lustre/tests/ost-pools.sh +++ b/lustre/tests/ost-pools.sh @@ -649,6 +649,95 @@ test_6() { } run_test 6 "getstripe/setstripe" +helper_test_7a() +{ + # Create a pool, stripe a directory and file with it + local pool=$1 + + pool_add $pool || error "pool_add failed" + pool_add_targets $pool 0 1 || error "pool_add_targets failed" + + $SETSTRIPE -c 1 $DIR/$tdir/testfile1 --pool "$pool" || \ + error "setstripe failed" + $SETSTRIPE -c 1 $DIR/$tdir/testfile2 --pool "$FSNAME.$pool" || \ + error "setstripe failed" + + mkdir $DIR/$tdir/testdir + $SETSTRIPE -c 1 $DIR/$tdir/testdir -p "$pool" || \ + error "setstripe failed" + $SETSTRIPE -c 1 $DIR/$tdir/testdir -p "$FSNAME.$pool" || \ + error "setstripe failed" + + rm -f $DIR/$tdir/testfile1 + rm -f $DIR/$tdir/testfile2 + rmdir $DIR/$tdir/testdir + + destroy_pool_int $FSNAME.$pool +} + +test_7a() +{ + [ $OSTCOUNT -lt 2 ] && skip "needs >= 2 OSTs" && return + + mkdir -p $DIR/$tdir + + # Generate pool with random name from 1 to 15 characters + for i in 1 9 15 ; do + POOLNAME=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w $i | + head -n 1) + echo set poolname to $POOLNAME + helper_test_7a $POOLNAME + done +} +run_test 7a "create various pool name" + +test_7b() +{ + # No fsname + do_facet mgs lctl pool_new qwerty + [ $? -ne 22 ] && error "can create a pool with no fsname" + + # No pool name + do_facet mgs lctl pool_new $FSNAME. + [ $? -ne 22 ] && error "can create a pool with no name" + + # Invalid character + do_facet mgs lctl pool_new $FSNAME.0123456789^bdef + [ $? -ne 22 ] && error "can create a pool with an invalid name" + + # Too long + do_facet mgs lctl pool_new $FSNAME.0123456789abdefg + [ $? -ne 36 ] && error "can create a pool with a name too long" + + return 0 +} +run_test 7b "try to create pool name with invalid lengths or names" + +test_7c() +{ + [ $OSTCOUNT -lt 2 ] && skip "needs >= 2 OSTs" && return + + mkdir -p $DIR/$tdir + + # Create a pool with 15 letters + local pool=0123456789abcde + pool_add $pool || error "pool_add failed" + pool_add_targets $pool 0 1 || error "pool_add_targets failed" + + # setstripe with the same pool name plus 1 letter + $SETSTRIPE -c 1 $DIR/$tdir/testfile1 --pool "${pool}X" && \ + error "setstripe succedeed" + + # setstripe with the same pool name minus 1 letter + $SETSTRIPE -c 1 $DIR/$tdir/testfile1 --pool "${pool%?}" && \ + error "setstripe succedeed" + + rm -f $DIR/$tdir/testfile1 + + destroy_pool_int $FSNAME.$pool +} +run_test 7c "create a valid pool name and setstripe with a bad one" + test_11() { set_cleanup_trap local POOL_ROOT=${POOL_ROOT:-$DIR/$tdir} diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh index 197ff9e..1dd2725 100755 --- a/lustre/tests/test-framework.sh +++ b/lustre/tests/test-framework.sh @@ -7055,7 +7055,7 @@ pool_add() { create_pool $FSNAME.$pool || { error_noexit "No pool created, result code $?"; return 1; } - [ $($LFS pool_list $FSNAME | grep -c $pool) -eq 1 ] || + [ $($LFS pool_list $FSNAME | grep -c "$FSNAME.${pool}\$") -eq 1 ] || { error_noexit "$pool not in lfs pool_list"; return 2; } } diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c index 767c5d4..2ce7586 100644 --- a/lustre/utils/lfs.c +++ b/lustre/utils/lfs.c @@ -73,6 +73,7 @@ #include #include #include +#include /* all functions */ static int lfs_setstripe(int argc, char **argv); @@ -1024,11 +1025,41 @@ static int lfs_setstripe(int argc, char **argv) return CMD_HELP; } - if (pool_name_arg && strlen(pool_name_arg) > LOV_MAXPOOLNAME) { - fprintf(stderr, - "error: %s: pool name '%s' is too long (max is %d characters)\n", - argv[0], pool_name_arg, LOV_MAXPOOLNAME); - return CMD_HELP; + if (pool_name_arg != NULL) { + char *ptr; + int rc; + + ptr = strchr(pool_name_arg, '.'); + if (ptr == NULL) { + ptr = pool_name_arg; + } else { + if ((ptr - pool_name_arg) == 0) { + fprintf(stderr, "error: %s: fsname is empty " + "in pool name '%s'\n", + argv[0], pool_name_arg); + return CMD_HELP; + } + + ++ptr; + } + + rc = lustre_is_poolname_valid(ptr, 1, LOV_MAXPOOLNAME); + if (rc == -1) { + fprintf(stderr, "error: %s: poolname '%s' is " + "empty\n", + argv[0], pool_name_arg); + return CMD_HELP; + } else if (rc == -2) { + fprintf(stderr, "error: %s: pool name '%s' is too long " + "(max is %d characters)\n", + argv[0], pool_name_arg, LOV_MAXPOOLNAME); + return CMD_HELP; + } else if (rc > 0) { + fprintf(stderr, "error: %s: char '%c' not allowed in " + "pool name '%s'\n", + argv[0], rc, pool_name_arg); + return CMD_HELP; + } } /* get the stripe size */ diff --git a/lustre/utils/mkfs_lustre.c b/lustre/utils/mkfs_lustre.c index bae26df..7ee492b 100644 --- a/lustre/utils/mkfs_lustre.c +++ b/lustre/utils/mkfs_lustre.c @@ -419,17 +419,19 @@ int parse_opts(int argc, char *const argv[], struct mkfs_opts *mop, sizeof(mop->mo_mkfsopts)); break; case 'L': { - char *tmp; - if ((strlen(optarg) < 1) || (strlen(optarg) > 8)) { - fprintf(stderr, "%s: filesystem name must be " - "1-8 chars\n", progname); - return 1; - } - if ((tmp = strpbrk(optarg, "/:"))) { - fprintf(stderr, "%s: char '%c' not allowed in " - "filesystem name\n", progname, *tmp); - return 1; - } + rc = lustre_is_fsname_valid(optarg, 1, + LUSTRE_MAXFSNAME); + if (rc < 0) { + fprintf(stderr, "%s: filesystem name must be " + "1-%d chars\n", progname, + LUSTRE_MAXFSNAME); + return 1; + } else if (rc > 0) { + fprintf(stderr, "%s: char '%c' not allowed in " + "filesystem name\n", progname, rc); + return 1; + } + strscpy(mop->mo_ldd.ldd_fsname, optarg, sizeof(mop->mo_ldd.ldd_fsname)); fsname_option = true; diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c index 37d639c..bb19f34 100644 --- a/lustre/utils/obd.c +++ b/lustre/utils/obd.c @@ -66,6 +66,7 @@ #include "obdctl.h" #include #include +#include #include #include @@ -75,6 +76,7 @@ #include #include +#include #define MAX_STRING_SIZE 128 #define DEVICES_LIST "/proc/fs/lustre/devices" @@ -4012,48 +4014,67 @@ err: return 0; } -static int extract_fsname_poolname(char *arg, char *fsname, char *poolname) +static int extract_fsname_poolname(const char *arg, char *fsname, + char *poolname) { - char *ptr; - int len; - int rc; + char *ptr; + int rc; - strcpy(fsname, arg); - ptr = strchr(fsname, '.'); - if (ptr == NULL) { - fprintf(stderr, ". is missing in %s\n", fsname); - rc = -EINVAL; - goto err; - } + strlcpy(fsname, arg, PATH_MAX + 1); + ptr = strchr(fsname, '.'); + if (ptr == NULL) { + fprintf(stderr, ". is missing in %s\n", fsname); + rc = -EINVAL; + goto err; + } - len = ptr - fsname; - if (len == 0) { - fprintf(stderr, "fsname is empty\n"); - rc = -EINVAL; - goto err; - } + if ((ptr - fsname) == 0) { + fprintf(stderr, "fsname is empty\n"); + rc = -EINVAL; + goto err; + } - len = strlen(ptr + 1); - if (len == 0) { - fprintf(stderr, "poolname is empty\n"); - rc = -EINVAL; - goto err; - } - if (len > LOV_MAXPOOLNAME) { - fprintf(stderr, - "poolname %s is too long (length is %d max is %d)\n", - ptr + 1, len, LOV_MAXPOOLNAME); - rc = -ENAMETOOLONG; - goto err; - } - strncpy(poolname, ptr + 1, LOV_MAXPOOLNAME); - poolname[LOV_MAXPOOLNAME] = '\0'; - *ptr = '\0'; - return 0; + *ptr = '\0'; + ++ptr; + + rc = lustre_is_fsname_valid(fsname, 1, LUSTRE_MAXFSNAME); + if (rc < 0) { + fprintf(stderr, "filesystem name %s must be 1-%d chars\n", + fsname, LUSTRE_MAXFSNAME); + rc = -EINVAL; + goto err; + } else if (rc > 0) { + fprintf(stderr, "char '%c' not allowed in filesystem name\n", + rc); + rc = -EINVAL; + goto err; + } + + rc = lustre_is_poolname_valid(ptr, 1, LOV_MAXPOOLNAME); + if (rc == -1) { + fprintf(stderr, "poolname is empty\n"); + rc = -EINVAL; + goto err; + } else if (rc == -2) { + fprintf(stderr, + "poolname %s is too long (max is %d)\n", + ptr, LOV_MAXPOOLNAME); + rc = -ENAMETOOLONG; + goto err; + } else if (rc > 0) { + fprintf(stderr, "char '%c' not allowed in pool name '%s'\n", + rc, ptr); + rc = -EINVAL; + goto err; + } + + strncpy(poolname, ptr, LOV_MAXPOOLNAME); + poolname[LOV_MAXPOOLNAME] = '\0'; + return 0; err: - fprintf(stderr, "argument %s must be .\n", arg); - return rc; + fprintf(stderr, "argument %s must be .\n", arg); + return rc; } int jt_pool_cmd(int argc, char **argv) -- 1.8.3.1