From: mwilck@suse.com
To: Christophe Varoqui <christophe.varoqui@opensvc.com>,
Benjamin Marzinski <bmarzins@redhat.com>
Cc: dm-devel@redhat.com, Martin Wilck <mwilck@suse.com>
Subject: [dm-devel] [PATCH 1/9] libmultipath: variable-size parameters in dm_get_map()
Date: Thu, 15 Jul 2021 12:52:15 +0200 [thread overview]
Message-ID: <20210715105223.30463-2-mwilck@suse.com> (raw)
In-Reply-To: <20210715105223.30463-1-mwilck@suse.com>
From: Martin Wilck <mwilck@suse.com>
We've seen a crash of multipath in disassemble_map because of a params
string exceeding PARAMS_SIZE. While the crash could have been fixed by
a simple error check, I believe multipath should be able to work with
arbitrary long parameter strings passed from the kernel.
The parameter list of dm_get_map() has changed. Bumped ABI version and
removed dm_get_map() and some functions from the ABI, which are only
called from libmultipath itself.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/devmapper.c | 44 ++++++++++++++++++++-----------
libmultipath/devmapper.h | 4 +--
libmultipath/libmultipath.version | 6 +----
libmultipath/structs_vec.c | 11 +++++---
4 files changed, 38 insertions(+), 27 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 945e625..a5194d8 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -648,7 +648,7 @@ int dm_map_present(const char * str)
return (do_get_info(str, &info) == 0);
}
-int dm_get_map(const char *name, unsigned long long *size, char *outparams)
+int dm_get_map(const char *name, unsigned long long *size, char **outparams)
{
int r = DMP_ERR;
struct dm_task *dmt;
@@ -682,12 +682,13 @@ int dm_get_map(const char *name, unsigned long long *size, char *outparams)
if (size)
*size = length;
- if (!outparams) {
+ if (!outparams)
r = DMP_OK;
- goto out;
+ else {
+ *outparams = strdup(params);
+ r = *outparams ? DMP_OK : DMP_ERR;
}
- if (snprintf(outparams, PARAMS_SIZE, "%s", params) <= PARAMS_SIZE)
- r = DMP_OK;
+
out:
dm_task_destroy(dmt);
return r;
@@ -761,7 +762,7 @@ is_mpath_part(const char *part_name, const char *map_name)
return 0;
}
-int dm_get_status(const char *name, char *outstatus)
+int dm_get_status(const char *name, char **outstatus)
{
int r = DMP_ERR;
struct dm_task *dmt;
@@ -799,8 +800,12 @@ int dm_get_status(const char *name, char *outstatus)
goto out;
}
- if (snprintf(outstatus, PARAMS_SIZE, "%s", status) <= PARAMS_SIZE)
+ if (!outstatus)
r = DMP_OK;
+ else {
+ *outstatus = strdup(status);
+ r = outstatus ? DMP_OK : DMP_ERR;
+ }
out:
if (r != DMP_OK)
condlog(0, "%s: error getting map status string", name);
@@ -1049,7 +1054,7 @@ int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove,
int queue_if_no_path = 0;
int udev_flags = 0;
unsigned long long mapsize;
- char params[PARAMS_SIZE] = {0};
+ char *params = NULL;
if (dm_is_mpath(mapname) != 1)
return 0; /* nothing to do */
@@ -1065,7 +1070,7 @@ int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove,
return 1;
if (need_suspend &&
- dm_get_map(mapname, &mapsize, params) == DMP_OK &&
+ dm_get_map(mapname, &mapsize, ¶ms) == DMP_OK &&
strstr(params, "queue_if_no_path")) {
if (!dm_queue_if_no_path(mapname, 0))
queue_if_no_path = 1;
@@ -1073,6 +1078,8 @@ int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove,
/* Leave queue_if_no_path alone if unset failed */
queue_if_no_path = -1;
}
+ free(params);
+ params = NULL;
if (dm_remove_partmaps(mapname, need_sync, deferred_remove))
return 1;
@@ -1431,7 +1438,7 @@ do_foreach_partmaps (const char * mapname,
struct dm_task *dmt;
struct dm_names *names;
unsigned next = 0;
- char params[PARAMS_SIZE];
+ char *params = NULL;
unsigned long long size;
char dev_t[32];
int r = 1;
@@ -1474,7 +1481,7 @@ do_foreach_partmaps (const char * mapname,
/*
* and we can fetch the map table from the kernel
*/
- dm_get_map(names->name, &size, ¶ms[0]) == DMP_OK &&
+ dm_get_map(names->name, &size, ¶ms) == DMP_OK &&
/*
* and the table maps over the multipath map
@@ -1486,12 +1493,15 @@ do_foreach_partmaps (const char * mapname,
goto out;
}
+ free(params);
+ params = NULL;
next = names->next;
names = (void *) names + next;
} while (next);
r = 0;
out:
+ free(params);
dm_task_destroy (dmt);
return r;
}
@@ -1620,17 +1630,19 @@ struct rename_data {
static int
rename_partmap (const char *name, void *data)
{
- char buff[PARAMS_SIZE];
+ char *buff = NULL;
int offset;
struct rename_data *rd = (struct rename_data *)data;
if (strncmp(name, rd->old, strlen(rd->old)) != 0)
return 0;
for (offset = strlen(rd->old); name[offset] && !(isdigit(name[offset])); offset++); /* do nothing */
- snprintf(buff, PARAMS_SIZE, "%s%s%s", rd->new, rd->delim,
- name + offset);
- dm_rename(name, buff, rd->delim, SKIP_KPARTX_OFF);
- condlog(4, "partition map %s renamed", name);
+ if (asprintf(&buff, "%s%s%s", rd->new, rd->delim, name + offset) >= 0) {
+ dm_rename(name, buff, rd->delim, SKIP_KPARTX_OFF);
+ free(buff);
+ condlog(4, "partition map %s renamed", name);
+ } else
+ condlog(1, "failed to rename partition map %s", name);
return 0;
}
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index e29b4d4..45a676d 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -44,8 +44,8 @@ int dm_addmap_create (struct multipath *mpp, char *params);
int dm_addmap_reload (struct multipath *mpp, char *params, int flush);
int dm_map_present (const char *);
int dm_map_present_by_uuid(const char *uuid);
-int dm_get_map(const char *, unsigned long long *, char *);
-int dm_get_status(const char *, char *);
+int dm_get_map(const char *, unsigned long long *, char **);
+int dm_get_status(const char *, char **);
int dm_type(const char *, char *);
int dm_is_mpath(const char *);
int _dm_flush_map (const char *, int, int, int, int);
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index 0cff311..7567837 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -31,7 +31,7 @@
* The new version inherits the previous ones.
*/
-LIBMULTIPATH_5.0.0 {
+LIBMULTIPATH_6.0.0 {
global:
/* symbols referenced by multipath and multipathd */
add_foreign;
@@ -58,8 +58,6 @@ global:
count_active_paths;
delete_all_foreign;
delete_foreign;
- disassemble_map;
- disassemble_status;
dlog;
dm_cancel_deferred_remove;
dm_enablegroup;
@@ -70,10 +68,8 @@ global:
dm_geteventnr;
dm_get_info;
dm_get_major_minor;
- dm_get_map;
dm_get_maps;
dm_get_multipath;
- dm_get_status;
dm_get_uuid;
dm_is_mpath;
dm_mapname;
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 7539019..24d6fd2 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -416,12 +416,12 @@ int
update_multipath_table (struct multipath *mpp, vector pathvec, int flags)
{
int r = DMP_ERR;
- char params[PARAMS_SIZE] = {0};
+ char *params = NULL;
if (!mpp)
return r;
- r = dm_get_map(mpp->alias, &mpp->size, params);
+ r = dm_get_map(mpp->alias, &mpp->size, ¶ms);
if (r != DMP_OK) {
condlog(2, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting table" : "map not present");
return r;
@@ -429,14 +429,17 @@ update_multipath_table (struct multipath *mpp, vector pathvec, int flags)
if (disassemble_map(pathvec, params, mpp)) {
condlog(2, "%s: cannot disassemble map", mpp->alias);
+ free(params);
return DMP_ERR;
}
- *params = '\0';
- if (dm_get_status(mpp->alias, params) != DMP_OK)
+ free(params);
+ params = NULL;
+ if (dm_get_status(mpp->alias, ¶ms) != DMP_OK)
condlog(2, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting status" : "map not present");
else if (disassemble_status(params, mpp))
condlog(2, "%s: cannot disassemble status", mpp->alias);
+ free(params);
/* FIXME: we should deal with the return value here */
update_pathvec_from_dm(pathvec, mpp, flags);
--
2.32.0
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2021-07-15 10:54 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-15 10:52 [dm-devel] [PATCH 0/9] multipath-tools: use variable-size string buffers mwilck
2021-07-15 10:52 ` mwilck [this message]
2021-07-26 22:17 ` [dm-devel] [PATCH 1/9] libmultipath: variable-size parameters in dm_get_map() Benjamin Marzinski
2021-08-11 14:18 ` Martin Wilck
2021-07-15 10:52 ` [dm-devel] [PATCH 2/9] libmultipath: strbuf: simple api for growing string buffers mwilck
2021-07-27 4:54 ` Benjamin Marzinski
2021-08-11 15:03 ` Martin Wilck
2021-07-15 10:52 ` [dm-devel] [PATCH 3/9] libmultipath: variable-size parameters in assemble_map() mwilck
2021-07-28 15:54 ` Benjamin Marzinski
2021-08-11 15:15 ` Martin Wilck
2021-07-15 10:52 ` [dm-devel] [PATCH 4/9] libmultipath: use strbuf in dict.c mwilck
2021-07-28 19:03 ` Benjamin Marzinski
2021-08-11 15:27 ` Martin Wilck
2021-08-30 18:23 ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 5/9] libmultipath: use strbuf in print.c mwilck
2021-07-29 0:00 ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 6/9] libmultipath: print.c: fail hard if keywords are not found mwilck
2021-07-29 2:36 ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 7/9] libmultipath: print.h: move macros to print.c mwilck
2021-07-29 2:36 ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 8/9] libmultipath: use strbuf in alias.c mwilck
2021-07-29 15:22 ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 9/9] multipathd: use strbuf in cli.c mwilck
2021-07-29 15:46 ` Benjamin Marzinski
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=20210715105223.30463-2-mwilck@suse.com \
--to=mwilck@suse.com \
--cc=bmarzins@redhat.com \
--cc=christophe.varoqui@opensvc.com \
--cc=dm-devel@redhat.com \
/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).