From ba56efcfdd53f3066d95bb2274f6f9cca9e34c0b Mon Sep 17 00:00:00 2001 From: James Simmons Date: Wed, 13 Sep 2017 21:21:22 -0400 Subject: [PATCH] LU-9767 utils: validate filesystem name for mkfs.lustre In an earlier LU-6401 patch various user land functions used to validate poolnames and file system names were removed. The checks instead were enforced on the kernel side to ensure any possible user land software directly interfacing to the kernel wouldn't be able to break things badly. For the case of formating the backend file system no kernel interaction doesn't happen until it tries to mount the MDT/OST/MGT which is very late in the process. So for this case lets add back the file system name verification to mkfs.lustre to warn users long before they try to mount anything. Secondly we remove the verify_poolname() in lfs.c since it really doesn't do much. Test-Parameters: trivial Lustre-commit: 28216a73f4c0b19ef45e3ec2d6949801f6b9f828 Lustre-change: https://review.whamcloud.com/28070 Change-Id: If094644e56a70b6dd8e6b0378adc8736911aeef1 Signed-off-by: James Simmons Reviewed-on: https://review.whamcloud.com/28070 Reviewed-by: Andreas Dilger Reviewed-by: John L. Hammond Reviewed-by: Fan Yong Reviewed-by: Oleg Drokin Reviewed-on: https://review.whamcloud.com/28996 Tested-by: Jenkins Tested-by: Maloo --- lustre/include/lustre/lustre_user.h | 2 ++ lustre/mgs/mgs_handler.c | 2 +- lustre/obdclass/obd_mount.c | 2 +- lustre/utils/lfs.c | 20 +------------------- lustre/utils/mkfs_lustre.c | 26 ++++++++++++++++++++++++-- lustre/utils/obd.c | 22 +++++++++++++++++++--- 6 files changed, 48 insertions(+), 26 deletions(-) diff --git a/lustre/include/lustre/lustre_user.h b/lustre/include/lustre/lustre_user.h index b1f3de3..de93d8b 100644 --- a/lustre/include/lustre/lustre_user.h +++ b/lustre/include/lustre/lustre_user.h @@ -686,6 +686,8 @@ static inline char *obd_uuid2str(const struct obd_uuid *uuid) return (char *)(uuid->uuid); } +#define LUSTRE_MAXFSNAME 8 + /* Extract fsname from uuid (or target name) of a target e.g. (myfs-OST0007_UUID -> myfs) see also deuuidify. */ diff --git a/lustre/mgs/mgs_handler.c b/lustre/mgs/mgs_handler.c index 6077b0d..585640b 100644 --- a/lustre/mgs/mgs_handler.c +++ b/lustre/mgs/mgs_handler.c @@ -762,7 +762,7 @@ static int mgs_extract_fs_pool(char *arg, char *fsname, char *poolname) return -EINVAL; /* or too long */ - if (len > MTI_NAME_MAXLEN) + if (len > LUSTRE_MAXFSNAME) return -ENAMETOOLONG; strncpy(fsname, arg, len); diff --git a/lustre/obdclass/obd_mount.c b/lustre/obdclass/obd_mount.c index f6797c1..21982b2 100644 --- a/lustre/obdclass/obd_mount.c +++ b/lustre/obdclass/obd_mount.c @@ -694,7 +694,7 @@ int server_name2fsname(const char *svname, char *fsname, const char **endptr) { const char *dash; - dash = svname + strnlen(svname, 8); /* max fsname length is 8 */ + dash = svname + strnlen(svname, LUSTRE_MAXFSNAME); for (; dash > svname && *dash != '-' && *dash != ':'; dash--) ; if (dash == svname) diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c index 5870cd8..cc6f8dd 100644 --- a/lustre/utils/lfs.c +++ b/lustre/utils/lfs.c @@ -1009,23 +1009,6 @@ static int parse_targets(__u32 *osts, int size, int offset, char *arg) return rc < 0 ? rc : nr; } -static int verify_pool_name(char *prog_name, char *pool_name) -{ - char *ptr; - - if (pool_name == NULL) - return 0; - - ptr = strchr(pool_name, '.'); - if (ptr != NULL && ptr == pool_name) { - fprintf(stderr, "error: %s: fsname is empty in pool name '%s'\n", - prog_name, pool_name); - return -EINVAL; - } - - return 0; -} - struct lfs_setstripe_args { unsigned long long lsa_comp_end; unsigned long long lsa_stripe_size; @@ -1555,8 +1538,7 @@ static int lfs_setstripe(int argc, char **argv) lsa.lsa_stripe_off = osts[0]; break; case 'p': - result = verify_pool_name(argv[0], optarg); - if (result) + if (optarg == NULL) goto error; lsa.lsa_pool_name = optarg; break; diff --git a/lustre/utils/mkfs_lustre.c b/lustre/utils/mkfs_lustre.c index 7bf83ea..7814ead 100644 --- a/lustre/utils/mkfs_lustre.c +++ b/lustre/utils/mkfs_lustre.c @@ -61,7 +61,7 @@ #include #include #include -#include +#include #include #include @@ -514,9 +514,31 @@ int parse_opts(int argc, char *const argv[], struct mkfs_opts *mop, ldd->ldd_flags &= ~LDD_F_NEED_INDEX; break; } - case 'L': + case 'L': { + const char *tmp; + size_t len; + + len = strlen(optarg); + if (len < 1 || len > LUSTRE_MAXFSNAME) { + fprintf(stderr, "%s: filesystem name must be " + "1-%d chars\n", progname, LUSTRE_MAXFSNAME); + return 1; + } + + for (tmp = optarg; *tmp != '\0'; ++tmp) { + if (isalnum(*tmp) || *tmp == '_' || *tmp == '-') + continue; + else + break; + } + if (*tmp != '\0') { + fprintf(stderr, "%s: char '%c' not allowed in " + "filesystem name\n", progname, *tmp); + return 1; + } strscpy(new_fsname, optarg, sizeof(new_fsname)); break; + } case 'm': { char *nids = convert_hostnames(optarg); diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c index ea1d45e..bea032f 100644 --- a/lustre/utils/obd.c +++ b/lustre/utils/obd.c @@ -3143,10 +3143,26 @@ static int pool_cmd(enum lcfg_command_type cmd, } rc = l_ioctl(OBD_DEV_ID, OBD_IOC_POOL, buf); out: - if (rc) - rc = -errno; + if (rc) + rc = -errno; + switch (rc) { + case ENAMETOOLONG: + fprintf(stderr, "error: %s: either the pool or file " + "system name is too long (max pool name len " + "is %d and file system name is %d)\n", + jt_cmdname(cmdname), LOV_MAXPOOLNAME, + LUSTRE_MAXFSNAME); + break; + case EINVAL: + fprintf(stderr, "error: %s can contain only " + "alphanumeric characters, underscores, and " + "dashes besides the required '.'\n", + jt_cmdname(cmdname)); + default: + break; + } free(lcfg); - return rc; + return rc; } /** -- 1.8.3.1