From 67417d9222c505103357191bb0e0ae300892e8a9 Mon Sep 17 00:00:00 2001 From: Mariusz Tkaczyk Date: Thu, 1 Jun 2023 09:27:48 +0200 Subject: [PATCH 155/165] mdadm: refactor ident->name handling Create dedicated setter for name in mddev_ident and propagate it. Following changes are made: - move duplicated code from config.c and mdadm.c into new function. - Add error enum in mdadm.h. - Use MD_NAME_MAX instead of hardcoded value in mddev_ident. - Use secure functions. - Add more detailed verification of the name. - make error messages reusable for cmdline and config: - for cmdline, these are errors so use pr_err(). - for config, these are just warnings, so use pr_info(). Signed-off-by: Mariusz Tkaczyk Signed-off-by: Jes Sorensen --- config.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++------ lib.c | 18 +++++++++++++ mdadm.c | 12 +++------ mdadm.h | 20 ++++++++++----- 4 files changed, 104 insertions(+), 23 deletions(-) diff --git a/config.c b/config.c index 450880e3..a9a0b4f7 100644 --- a/config.c +++ b/config.c @@ -131,6 +131,34 @@ bool is_devname_ignore(char *devname) return false; } +/** + * ident_log() - generate and write message to the user. + * @param_name: name of the property. + * @value: value of the property. + * @reason: meaningful description. + * @cmdline: context dependent actions, see below. + * + * The function is made to provide similar error handling for both config and cmdline. The behavior + * is configurable via @cmdline. Message has following format: + * "Value "@value" cannot be set for @param_name. Reason: @reason." + * + * If cmdline is on: + * - message is written to stderr. + * otherwise: + * - message is written to stdout. + * - "Value ignored" is added at the end of the message. + */ +static void ident_log(const char *param_name, const char *value, const char *reason, + const bool cmdline) +{ + if (cmdline == true) + pr_err("Value \"%s\" cannot be set as %s. Reason: %s.\n", value, param_name, + reason); + else + pr_info("Value \"%s\" cannot be set as %s. Reason: %s. Value ignored.\n", value, + param_name, reason); +} + /** * ident_init() - Set defaults. * @ident: ident pointer, not NULL. @@ -159,6 +187,46 @@ 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. + * @cmdline: context dependent actions. + * + * If criteria passed, set name in @ident. + * + * Return: %MDADM_STATUS_SUCCESS or %MDADM_STATUS_ERROR. + */ +static mdadm_status_t _ident_set_name(struct mddev_ident *ident, const char *name, + const bool cmdline) +{ + assert(name); + assert(ident); + + const char *prop_name = "name"; + + if (ident->name[0]) { + ident_log(prop_name, name, "Already defined", cmdline); + return MDADM_STATUS_ERROR; + } + + if (is_string_lq(name, MD_NAME_MAX + 1) == false) { + ident_log(prop_name, name, "Too long or empty", cmdline); + return MDADM_STATUS_ERROR; + } + + snprintf(ident->name, MD_NAME_MAX + 1, "%s", name); + return MDADM_STATUS_SUCCESS; +} + +/** + * ident_set_name()- exported, for cmdline. + */ +mdadm_status_t ident_set_name(struct mddev_ident *ident, const char *name) +{ + return _ident_set_name(ident, name, true); +} + struct conf_dev { struct conf_dev *next; char *name; @@ -444,14 +512,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, false); } 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 8a4b48e0..03198e2d 100644 --- a/lib.c +++ b/lib.c @@ -27,6 +27,24 @@ #include #include +/** + * is_string_lq() - Check if string length with NULL byte 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_string_lq(const char * const str, size_t max_len) +{ + assert(str); + + size_t _len = strnlen(str, max_len); + + if (_len > 0 && _len < max_len) + return true; + return false; +} + bool is_dev_alive(char *path) { if (!path) diff --git a/mdadm.c b/mdadm.c index 0a56ed26..8bc9304e 100644 --- a/mdadm.c +++ b/mdadm.c @@ -690,20 +690,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) != MDADM_STATUS_SUCCESS) exit(2); - } - strcpy(ident.name, optarg); + continue; case O(ASSEMBLE,'m'): /* super-minor for array */ diff --git a/mdadm.h b/mdadm.h index 5678eb11..bbf386d6 100644 --- a/mdadm.h +++ b/mdadm.h @@ -294,6 +294,11 @@ static inline void __put_unaligned32(__u32 val, void *p) #define KIB_TO_BYTES(x) ((x) << 10) #define SEC_TO_BYTES(x) ((x) << 9) +/** + * This is true for native and DDF, IMSM allows 16. + */ +#define MD_NAME_MAX 32 + extern const char Name[]; struct md_bb_entry { @@ -425,6 +430,12 @@ struct spare_criteria { unsigned int sector_size; }; +typedef enum mdadm_status { + MDADM_STATUS_SUCCESS = 0, + MDADM_STATUS_ERROR, + MDADM_STATUS_UNDEF, +} mdadm_status_t; + enum mode { ASSEMBLE=1, BUILD, @@ -593,7 +604,7 @@ struct mddev_ident { int uuid_set; int uuid[4]; - char name[33]; + char name[MD_NAME_MAX + 1]; int super_minor; @@ -1609,6 +1620,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_string_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); @@ -1626,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_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); @@ -2002,11 +2015,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 -- 2.40.1