From 28216a73f4c0b19ef45e3ec2d6949801f6b9f828 Mon Sep 17 00:00:00 2001 From: James Simmons Date: Sun, 20 Aug 2017 23:24:20 -0400 Subject: [PATCH] LU-9767 utils: validate filesystem name for mkfs.lustre The patch "LU-6401 uapi: turn lustre_param.h into a proper UAPI header" removed 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 duplicates extract_fsname_poolname() in obd.c. Their is no need to do the same test twice. The function pool_cmd() calls the ioctl for pool handling which in turn returns an error code. Use this error code to notify the user what mistake they did for their pool command. For the MGS kernel code mgs_extract_fs_pool() was checking MTI_NAME_MAXLEN instead of LUSTRE_MAXFSNAME. Also use LUSTRE_MAXFSNAME instead of the raw number in the function server_name2fsname() located in obd_mount.c. 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 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/include/uapi/linux/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 | 18 +++++++++++++++++- 6 files changed, 46 insertions(+), 24 deletions(-) diff --git a/lustre/include/uapi/linux/lustre/lustre_user.h b/lustre/include/uapi/linux/lustre/lustre_user.h index 83c45d3..17c985a 100644 --- a/lustre/include/uapi/linux/lustre/lustre_user.h +++ b/lustre/include/uapi/linux/lustre/lustre_user.h @@ -687,6 +687,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 dc13c3d..d50b594 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 6388b87..362d8c1 100644 --- a/lustre/obdclass/obd_mount.c +++ b/lustre/obdclass/obd_mount.c @@ -693,7 +693,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 d758248..36a67f3 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; @@ -1549,8 +1532,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 a9faace..0b48ad8 100644 --- a/lustre/utils/mkfs_lustre.c +++ b/lustre/utils/mkfs_lustre.c @@ -62,7 +62,7 @@ #include #include #include -#include +#include #include #include "mount_utils.h" @@ -518,9 +518,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 db5d248..33b67ca 100644 --- a/lustre/utils/obd.c +++ b/lustre/utils/obd.c @@ -3149,8 +3149,24 @@ static int pool_cmd(enum lcfg_command_type cmd, out: 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