linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
To: jes@trained-monkey.org, colyli@suse.de
Cc: linux-raid@vger.kernel.org
Subject: [PATCH 2/3] mdadm: refactor ident->name handling
Date: Wed, 21 Dec 2022 12:50:18 +0100	[thread overview]
Message-ID: <20221221115019.26276-3-mariusz.tkaczyk@linux.intel.com> (raw)
In-Reply-To: <20221221115019.26276-1-mariusz.tkaczyk@linux.intel.com>

Move duplicated code for both config.c and mdadm.c to new functions.
Add error enum in mdadm.h. Use MD_NAME_MAX instead hardcoded value
in mddev_ident. Use secure functions.

In next patch POSIX validation is added.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 config.c | 35 +++++++++++++++++++++++++++--------
 lib.c    | 16 ++++++++++++++++
 mdadm.c  | 16 ++++------------
 mdadm.h  | 21 +++++++++++++++------
 util.c   | 20 ++++++++++++++++++++
 5 files changed, 82 insertions(+), 26 deletions(-)

diff --git a/config.c b/config.c
index eeedd0c6..7d3eb6fc 100644
--- a/config.c
+++ b/config.c
@@ -147,6 +147,32 @@ inline void ident_init(struct mddev_ident *ident)
 	ident->uuid_set = 0;
 }
 
+/**
+ * ident_set_name()- set name in &mddev_ident.
+ * @ident: pointer to &mddev_ident.
+ * @name: name to be set.
+ *
+ * If criteria passed, set name in @ident.
+ *
+ * Return: %STATUS_SUCCESS or %STATUS_ERROR.
+ */
+status_t ident_set_name(struct mddev_ident *ident, const char *name)
+{
+	assert(name);
+	assert(ident);
+
+	if (ident->name[0]) {
+		pr_err("Name can be specified once, second value is '%s'.\n", name);
+		return STATUS_ERROR;
+	}
+
+	if (check_mdname(name) == STATUS_ERROR)
+		return STATUS_ERROR;
+
+	snprintf(ident->name, MD_NAME_MAX + 1, "%s", name);
+	return STATUS_SUCCESS;
+}
+
 struct conf_dev {
 	struct conf_dev *next;
 	char *name;
@@ -445,14 +471,7 @@ void arrayline(char *line)
 					mis.super_minor = minor;
 			}
 		} else if (strncasecmp(w, "name=", 5) == 0) {
-			if (mis.name[0])
-				pr_err("only specify name once, %s ignored.\n",
-					w);
-			else if (strlen(w + 5) > 32)
-				pr_err("name too long, ignoring %s\n", w);
-			else
-				strcpy(mis.name, w + 5);
-
+			ident_set_name(&mis, w + 5);
 		} else if (strncasecmp(w, "bitmap=", 7) == 0) {
 			if (mis.bitmap_file)
 				pr_err("only specify bitmap file once. %s ignored\n",
diff --git a/lib.c b/lib.c
index e395b28d..624580e1 100644
--- a/lib.c
+++ b/lib.c
@@ -27,6 +27,22 @@
 #include	<ctype.h>
 #include	<limits.h>
 
+/**
+ * is_strnlen_lq() - Check if string length is lower or equal to requested.
+ * @str: string to check.
+ * @max_len: max length.
+ *
+ * @str length must be bigger than 0 and be lower or equal @max_len, including termination byte.
+ */
+bool is_strnlen_lq(const char * const str, size_t max_len)
+{
+	assert(str);
+
+	size_t _len = strnlen(str, max_len);
+
+	return (_len < max_len && _len != 0);
+}
+
 bool is_dev_alive(char *path)
 {
 	if (!path)
diff --git a/mdadm.c b/mdadm.c
index 74fdec31..5bd3d5a7 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -686,20 +686,14 @@ int main(int argc, char *argv[])
 		case O(CREATE,'N'):
 		case O(ASSEMBLE,'N'):
 		case O(MISC,'N'):
-			if (ident.name[0]) {
-				pr_err("name cannot be set twice.   Second value %s.\n", optarg);
-				exit(2);
-			}
 			if (mode == MISC && !c.subarray) {
 				pr_err("-N/--name only valid with --update-subarray in misc mode\n");
 				exit(2);
 			}
-			if (strlen(optarg) > 32) {
-				pr_err("name '%s' is too long, 32 chars max.\n",
-					optarg);
+
+			if (ident_set_name(&ident, optarg) != STATUS_SUCCESS)
 				exit(2);
-			}
-			strcpy(ident.name, optarg);
+
 			continue;
 
 		case O(ASSEMBLE,'m'): /* super-minor for array */
@@ -1341,10 +1335,8 @@ int main(int argc, char *argv[])
 		} else {
 			char *bname = basename(devlist->devname);
 
-			if (strlen(bname) > MD_NAME_MAX) {
-				pr_err("Name %s is too long.\n", devlist->devname);
+			if (check_mdname(bname))
 				exit(1);
-			}
 
 			ret = stat(devlist->devname, &stb);
 			if (ident.super_minor == -2 && ret != 0) {
diff --git a/mdadm.h b/mdadm.h
index 23ffe977..b68fa4bc 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -275,6 +275,11 @@ static inline void __put_unaligned32(__u32 val, void *p)
 
 #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
 
+/**
+ * This is true for native and DDF, IMSM allows 16.
+ */
+#define MD_NAME_MAX 32
+
 extern const char Name[];
 
 struct md_bb_entry {
@@ -406,6 +411,12 @@ struct spare_criteria {
 	unsigned int sector_size;
 };
 
+typedef enum status {
+	STATUS_SUCCESS = 0,
+	STATUS_ERROR,
+	STATUS_UNDEF,
+} status_t;
+
 enum mode {
 	ASSEMBLE=1,
 	BUILD,
@@ -528,7 +539,7 @@ struct mddev_ident {
 
 	int	uuid_set;
 	int	uuid[4];
-	char	name[33];
+	char	name[MD_NAME_MAX + 1];
 
 	int super_minor;
 
@@ -1538,6 +1549,7 @@ extern int check_partitions(int fd, char *dname,
 extern int fstat_is_blkdev(int fd, char *devname, dev_t *rdev);
 extern int stat_is_blkdev(char *devname, dev_t *rdev);
 
+extern bool is_strnlen_lq(const char * const str, size_t max_len);
 extern bool is_dev_alive(char *path);
 extern int get_mdp_major(void);
 extern int get_maj_min(char *dev, int *major, int *minor);
@@ -1555,6 +1567,7 @@ extern void manage_fork_fds(int close_all);
 extern int continue_via_systemd(char *devnm, char *service_name);
 
 extern void ident_init(struct mddev_ident *ident);
+extern status_t ident_set_name(struct mddev_ident *ident, const char *name);
 
 extern int parse_auto(char *str, char *msg, int config);
 extern struct mddev_ident *conf_get_ident(char *dev);
@@ -1634,6 +1647,7 @@ extern void print_r10_layout(int layout);
 
 extern char *find_free_devnm(int use_partitions);
 
+extern error_t check_mdname(const char *name);
 extern void put_md_name(char *name);
 extern char *devid2kname(dev_t devid);
 extern char *devid2devnm(dev_t devid);
@@ -1923,11 +1937,6 @@ enum r0layout {
 /* And another special number needed for --data_offset=variable */
 #define VARIABLE_OFFSET 3
 
-/**
- * This is true for native and DDF, IMSM allows 16.
- */
-#define MD_NAME_MAX 32
-
 /**
  * is_container() - check if @level is &LEVEL_CONTAINER
  * @level: level value
diff --git a/util.c b/util.c
index 26ffdcea..b2a4ea21 100644
--- a/util.c
+++ b/util.c
@@ -932,6 +932,26 @@ int get_data_disks(int level, int layout, int raid_disks)
 	return data_disks;
 }
 
+/**
+ * check_md_name()- check if MD device name is correct.
+ * @name: name to check.
+ *
+ * In case of error, message is printed.
+ *
+ * Return: %STATUS_SUCCESS or %STATUS_ERROR.
+ */
+error_t check_mdname(const char * const name)
+{
+	assert(name);
+
+	if (!is_strnlen_lq(name, MD_NAME_MAX + 1)) {
+		pr_err("Name '%s' is too long or empty, %d characters max.\n", name, MD_NAME_MAX);
+		return STATUS_ERROR;
+	}
+
+	return STATUS_SUCCESS;
+}
+
 dev_t devnm2devid(char *devnm)
 {
 	/* First look in /sys/block/$DEVNM/dev for %d:%d
-- 
2.26.2


  parent reply	other threads:[~2022-12-21 11:50 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-21 11:50 [PATCH 0/3] Validation for names during creation Mariusz Tkaczyk
2022-12-21 11:50 ` [PATCH 1/3] mdadm: create ident_init() Mariusz Tkaczyk
2022-12-28 15:05   ` Jes Sorensen
2022-12-21 11:50 ` Mariusz Tkaczyk [this message]
2022-12-28 15:07   ` [PATCH 2/3] mdadm: refactor ident->name handling Jes Sorensen
2022-12-29  9:39     ` Mariusz Tkaczyk
2023-01-09 10:51       ` Mariusz Tkaczyk
2023-03-02 14:52       ` Jes Sorensen
2023-03-03 12:04         ` Mariusz Tkaczyk
2023-03-08 19:04           ` Jes Sorensen
2023-03-09  8:02             ` Mariusz Tkaczyk
2023-03-10 14:43               ` Jes Sorensen
2022-12-21 11:50 ` [PATCH 3/3] Limit length and set of characters allowed of devname Mariusz Tkaczyk
2023-03-13 14:22   ` Jes Sorensen
2023-03-14  8:14     ` Mariusz Tkaczyk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221221115019.26276-3-mariusz.tkaczyk@linux.intel.com \
    --to=mariusz.tkaczyk@linux.intel.com \
    --cc=colyli@suse.de \
    --cc=jes@trained-monkey.org \
    --cc=linux-raid@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).