From: Martin Wilck <mwilck@xxxxxxxx>
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@xxxxxxxx>
---
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..c05dc20 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@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
