mdadm: define ident_set_devname()

Use dedicated set method for ident->devname. Now, devname validation
is done early for modes where device is created (Build, Create and
Assemble). The rules, used for devname validation are derived from
config file.

It could cause regression with execeptional cases where existing device
has name which doesn't match criteria for Manage and Grow modes. It is
low risk and those modes are not omitted from early devname validation.
Use can used main numbered devnode to avoid this problem.
Messages exposed to user are changed so it might cause a regression
in negative scenarios. Error codes are not changed.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
Signed-off-by: Jes Sorensen <jes@trained-monkey.org>

diff --git a/config.c b/config.c
index a9a0b4f7b6f075a5600a205802a4ed1062cdb87a..a0424845a0cddb8ca3d33f14101e6d00d8b5c0d8 100644 (file)
--- a/config.c
+++ b/config.c
@@ -122,7 +122,7 @@ int match_keyword(char *word)
 /**
  * is_devname_ignore() - check if &devname is a special "<ignore>" keyword.
  */
-bool is_devname_ignore(char *devname)
+bool is_devname_ignore(const char *devname)
 {
        static const char keyword[] = "<ignore>";
 
@@ -187,6 +187,74 @@ inline void ident_init(struct mddev_ident *ident)
        ident->uuid_set = 0;
 }
 
+/**
+ * _ident_set_devname()- verify devname and set it in &mddev_ident.
+ * @ident: pointer to &mddev_ident.
+ * @devname: devname to be set.
+ * @cmdline: context dependent actions. If set, ignore keyword is not allowed.
+ *
+ * @devname can have following forms:
+ *     '<ignore>' keyword (if allowed)
+ *     /dev/md{number}
+ *     /dev/md_d{number} (legacy)
+ *     /dev/md_{name}
+ *     /dev/md/{name}
+ *     {name} - anything that doesn't start from '/' or '<'.
+ *
+ * {name} must follow name's criteria.
+ * If criteria passed, duplicate memory and set devname in @ident.
+ *
+ * Return: %MDADM_STATUS_SUCCESS or %MDADM_STATUS_ERROR.
+ */
+mdadm_status_t _ident_set_devname(struct mddev_ident *ident, const char *devname,
+                                 const bool cmdline)
+{
+       assert(ident);
+       assert(devname);
+
+       static const char named_dev_pref[] = DEV_NUM_PREF "_";
+       static const int named_dev_pref_size = sizeof(named_dev_pref) - 1;
+       const char *prop_name = "devname";
+       const char *name;
+
+       if (ident->devname) {
+               ident_log(prop_name, devname, "Already defined", cmdline);
+               return MDADM_STATUS_ERROR;
+       }
+
+       if (is_devname_ignore(devname) == true) {
+               if (!cmdline)
+                       goto pass;
+
+               ident_log(prop_name, devname, "Special keyword is invalid in this context",
+                         cmdline);
+               return MDADM_STATUS_ERROR;
+       }
+
+       if (is_devname_md_numbered(devname) == true || is_devname_md_d_numbered(devname) == true)
+               goto pass;
+
+       if (strncmp(devname, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0)
+               name = devname + DEV_MD_DIR_LEN;
+       else if (strncmp(devname, named_dev_pref, named_dev_pref_size) == 0)
+               name = devname + named_dev_pref_size;
+       else
+               name = devname;
+
+       if (*name == '/' || *name == '<') {
+               ident_log(prop_name, devname, "Cannot be started from \'/\' or \'<\'", cmdline);
+               return MDADM_STATUS_ERROR;
+       }
+
+       if (is_string_lq(name, MD_NAME_MAX + 1) == false) {
+               ident_log(prop_name, devname, "Invalid length", cmdline);
+               return MDADM_STATUS_ERROR;
+       }
+pass:
+       ident->devname = xstrdup(devname);
+       return MDADM_STATUS_SUCCESS;
+}
+
 /**
  * _ident_set_name()- set name in &mddev_ident.
  * @ident: pointer to &mddev_ident.
@@ -219,6 +287,14 @@ static mdadm_status_t _ident_set_name(struct mddev_ident *ident, const char *nam
        return MDADM_STATUS_SUCCESS;
 }
 
+/**
+ * ident_set_devname()- exported, for cmdline.
+ */
+mdadm_status_t ident_set_devname(struct mddev_ident *ident, const char *name)
+{
+       return _ident_set_devname(ident, name, true);
+}
+
 /**
  * ident_set_name()- exported, for cmdline.
  */
@@ -464,29 +540,7 @@ void arrayline(char *line)
 
        for (w = dl_next(line); w != line; w = dl_next(w)) {
                if (w[0] == '/' || strchr(w, '=') == NULL) {
-                       /* This names the device, or is '<ignore>'.
-                        * The rules match those in create_mddev.
-                        * 'w' must be:
-                        *  /dev/md/{anything}
-                        *  /dev/mdNN
-                        *  /dev/md_dNN
-                        *  <ignore>
-                        *  or anything that doesn't start '/' or '<'
-                        */
-                       if (is_devname_ignore(w) == true ||
-                           strncmp(w, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0 ||
-                           (w[0] != '/' && w[0] != '<') ||
-                           is_devname_md_numbered(w) == true ||
-                           is_devname_md_d_numbered(w) == true) {
-                               /* This is acceptable */;
-                               if (mis.devname)
-                                       pr_err("only give one device per ARRAY line: %s and %s\n",
-                                               mis.devname, w);
-                               else
-                                       mis.devname = w;
-                       }else {
-                               pr_err("%s is an invalid name for an md device - ignored.\n", w);
-                       }
+                       _ident_set_devname(&mis, w, false);
                } else if (strncasecmp(w, "uuid=", 5) == 0) {
                        if (mis.uuid_set)
                                pr_err("only specify uuid once, %s ignored.\n",

diff --git a/mdadm.c b/mdadm.c
index 8bc9304ef6d070586ded7778feda1d95d7079bc5..62f981dfbd15f0efea2a065552c983c007f2205c 100644 (file)
--- a/mdadm.c
+++ b/mdadm.c
@@ -1284,7 +1284,8 @@ int main(int argc, char *argv[])
                        pr_err("an md device must be given in this mode\n");
                        exit(2);
                }
-               ident.devname = devlist->devname;
+               if (ident_set_devname(&ident, devlist->devname) != MDADM_STATUS_SUCCESS)
+                       exit(1);
 
                if ((int)ident.super_minor == -2 && c.autof) {
                        pr_err("--super-minor=dev is incompatible with --auto\n");
@@ -1301,13 +1302,6 @@ int main(int argc, char *argv[])
                                exit(1);
                        }
                } else {
-                       char *bname = basename(ident.devname);
-
-                       if (strlen(bname) > MD_NAME_MAX) {
-                               pr_err("Name %s is too long.\n", ident.devname);
-                               exit(1);
-                       }
-
                        ret = stat(ident.devname, &stb);
                        if (ident.super_minor == -2 && ret != 0) {
                                pr_err("--super-minor=dev given, and listed device %s doesn't exist.\n",

diff --git a/mdadm.h b/mdadm.h
index bbf386d6cd77186ea4cb76a1b48ab03d6c23c2de..49422e241ba8cd102d84fb3f76c0cfc9f94034e1 100644 (file)
--- a/mdadm.h
+++ b/mdadm.h
@@ -1638,6 +1638,7 @@ extern void manage_fork_fds(int close_all);
 extern int continue_via_systemd(char *devnm, char *service_name, char *prefix);
 
 extern void ident_init(struct mddev_ident *ident);
+extern mdadm_status_t ident_set_devname(struct mddev_ident *ident, const char *devname);
 extern mdadm_status_t ident_set_name(struct mddev_ident *ident, const char *name);
 
 extern int parse_auto(char *str, char *msg, int config);
@@ -1660,7 +1661,7 @@ extern void print_escape(char *str);
 extern int use_udev(void);
 extern unsigned long GCD(unsigned long a, unsigned long b);
 extern int conf_name_is_free(char *name);
-extern bool is_devname_ignore(char *devname);
+extern bool is_devname_ignore(const char *devname);
 extern bool is_devname_md_numbered(const char *devname);
 extern bool is_devname_md_d_numbered(const char *devname);
 extern int conf_verify_devnames(struct mddev_ident *array_list);

diff --git a/tests/00createnames b/tests/00createnames
index 064eeef2895037a3c7da5c4d6185b23bd834199a..a95e7d2bb0855d5377439fe26f76ffa17369d339 100644 (file)
--- a/tests/00createnames
+++ b/tests/00createnames
@@ -39,3 +39,6 @@ mdadm -S "/dev/md0"
 names_create "/dev/md0" "name"
 names_verify "/dev/md0" "empty" "name"
 mdadm -S "/dev/md0"
+
+# Devnode is a special ignore keyword. Should be rejected.
+names_create "<ignore>" "name", "true"

diff --git a/tests/templates/names_template b/tests/templates/names_template
index 8d2b5c81c10b83d1354ce949736c2754f213513f..6181bfaaaa1a2c7b74da72d84fb7cce82b909d61 100644 (file)
--- a/tests/templates/names_template
+++ b/tests/templates/names_template
@@ -2,6 +2,7 @@
 function names_create() {
        local DEVNAME=$1
        local NAME=$2
+       local NEG_TEST=$3
 
        if [[ -z "$NAME" ]]; then
                mdadm -CR "$DEVNAME" -l0 -n 1 $dev0 --force
@@ -9,6 +10,12 @@ function names_create() {
                mdadm -CR "$DEVNAME" --name="$NAME" --metadata=1.2 -l0 -n 1 $dev0 --force
        fi
 
+       if [[ "$NEG_TEST" == "true" ]]; then
+               [[ "$?" == "0" ]] && return 0
+               echo "Negative verification failed"
+               exit 1
+       fi
+
        if [[ "$?" != "0" ]]; then
                echo "Cannot create device."
                exit 1