diff --git a/0011-multipathd-don-t-fail-to-remove-path-once-the-map-is.patch b/0011-multipathd-don-t-fail-to-remove-path-once-the-map-is.patch new file mode 100644 index 0000000..24c9043 --- /dev/null +++ b/0011-multipathd-don-t-fail-to-remove-path-once-the-map-is.patch @@ -0,0 +1,69 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 11 May 2021 11:15:47 -0500 +Subject: [PATCH] multipathd: don't fail to remove path once the map is removed + +In ev_remove_path(), if update_mpp_paths() fails, we delete the entire +map. However, since update_mpp_paths() happens before we call +set_path_removed(), pp->initialized isn't set to INIT_REMOVED, so +remove_map_and_stop_waiter() doesn't remove the path when in removes the +map. But with the map removed, there's nothing to keep us from removing +the path. + +Call set_path_removed() before update_mpp_paths() to avoid the odd case +of ev_remove_path() removing the map but not the path. + +Signed-off-by: Benjamin Marzinski +--- + libmultipath/structs_vec.c | 4 ++-- + multipathd/main.c | 13 ++++++++----- + 2 files changed, 10 insertions(+), 7 deletions(-) + +diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c +index d242c06b..75390198 100644 +--- a/libmultipath/structs_vec.c ++++ b/libmultipath/structs_vec.c +@@ -45,8 +45,8 @@ int update_mpp_paths(struct multipath *mpp, vector pathvec) + + /* + * Avoid adding removed paths to the map again +- * when we reload it. Such paths may exist if +- * domap fails in ev_remove_path(). ++ * when we reload it. Such paths may exist in ++ * ev_remove_paths() or if it returns failure. + */ + pp1 = find_path_by_devt(pathvec, pp->dev_t); + if (pp1 && pp->initialized != INIT_REMOVED && +diff --git a/multipathd/main.c b/multipathd/main.c +index 102946bf..449ce384 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -1199,6 +1199,13 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) + * avoid referring to the map of an orphaned path + */ + if ((mpp = pp->mpp)) { ++ /* ++ * Mark the path as removed. In case of success, we ++ * will delete it for good. Otherwise, it will be deleted ++ * later, unless all attempts to reload this map fail. ++ */ ++ set_path_removed(pp); ++ + /* + * transform the mp->pg vector of vectors of paths + * into a mp->params string to feed the device-mapper +@@ -1210,13 +1217,9 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) + } + + /* +- * Mark the path as removed. In case of success, we +- * will delete it for good. Otherwise, it will be deleted +- * later, unless all attempts to reload this map fail. +- * Note: we have to explicitly remove pp from mpp->paths, ++ * we have to explicitly remove pp from mpp->paths, + * update_mpp_paths() doesn't do that. + */ +- set_path_removed(pp); + i = find_slot(mpp->paths, pp); + if (i != -1) + vector_del_slot(mpp->paths, i); diff --git a/0012-multipathd-remove-duplicate-orphan_paths-in-flush_ma.patch b/0012-multipathd-remove-duplicate-orphan_paths-in-flush_ma.patch new file mode 100644 index 0000000..38b1d2b --- /dev/null +++ b/0012-multipathd-remove-duplicate-orphan_paths-in-flush_ma.patch @@ -0,0 +1,26 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 11 May 2021 11:55:14 -0500 +Subject: [PATCH] multipathd: remove duplicate orphan_paths in flush_map + +remove_map_and_stop_waiter() already calls orphan_paths() so flush_map() +doesn't need to call orphan_paths() before calling +remove_map_and_stop_waiter(). + +Signed-off-by: Benjamin Marzinski +--- + multipathd/main.c | 1 - + 1 file changed, 1 deletion(-) + +diff --git a/multipathd/main.c b/multipathd/main.c +index 449ce384..6090434c 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -660,7 +660,6 @@ flush_map(struct multipath * mpp, struct vectors * vecs, int nopaths) + else + condlog(2, "%s: map flushed", mpp->alias); + +- orphan_paths(vecs->pathvec, mpp, "map flushed"); + remove_map_and_stop_waiter(mpp, vecs); + + return 0; diff --git a/0013-multipathd-fix-ev_remove_path-return-code-handling.patch b/0013-multipathd-fix-ev_remove_path-return-code-handling.patch new file mode 100644 index 0000000..c9e5333 --- /dev/null +++ b/0013-multipathd-fix-ev_remove_path-return-code-handling.patch @@ -0,0 +1,225 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 11 May 2021 13:58:57 -0500 +Subject: [PATCH] multipathd: fix ev_remove_path return code handling + +When ev_remove_path() returned success, callers assumed that the path +(and possibly the map) had been removed. When ev_remove_path() returned +failure, callers assumed that the path had not been removed. However, +the path could be removed on both success or failure. This could cause +callers to dereference the path after it was removed. + +To deal with this, make ev_remove_path() return a different symbolic +value for each outcome, and make the callers react appropriately for +the different values. Found by coverity. + +Signed-off-by: Benjamin Marzinski +--- + multipathd/cli_handlers.c | 24 +++++++++++++++++++++-- + multipathd/main.c | 41 ++++++++++++++++++++------------------- + multipathd/main.h | 14 +++++++++++++ + 3 files changed, 57 insertions(+), 22 deletions(-) + +diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c +index 1de6ad8e..6765fcf0 100644 +--- a/multipathd/cli_handlers.c ++++ b/multipathd/cli_handlers.c +@@ -752,7 +752,8 @@ cli_add_path (void * v, char ** reply, int * len, void * data) + /* Have the checker reinstate this path asap */ + pp->tick = 1; + return 0; +- } else if (!ev_remove_path(pp, vecs, true)) ++ } else if (ev_remove_path(pp, vecs, true) & ++ REMOVE_PATH_SUCCESS) + /* Path removed in ev_remove_path() */ + pp = NULL; + else { +@@ -813,6 +814,7 @@ cli_del_path (void * v, char ** reply, int * len, void * data) + struct vectors * vecs = (struct vectors *)data; + char * param = get_keyparam(v, PATH); + struct path *pp; ++ int ret; + + param = convert_dev(param, 1); + condlog(2, "%s: remove path (operator)", param); +@@ -821,7 +823,25 @@ cli_del_path (void * v, char ** reply, int * len, void * data) + condlog(0, "%s: path already removed", param); + return 1; + } +- return ev_remove_path(pp, vecs, 1); ++ ret = ev_remove_path(pp, vecs, 1); ++ if (ret == REMOVE_PATH_DELAY) { ++ *reply = strdup("delayed\n"); ++ if (*reply) ++ *len = strlen(*reply) + 1; ++ else { ++ *len = 0; ++ ret = REMOVE_PATH_FAILURE; ++ } ++ } else if (ret == REMOVE_PATH_MAP_ERROR) { ++ *reply = strdup("map reload error. removed\n"); ++ if (*reply) ++ *len = strlen(*reply) + 1; ++ else { ++ *len = 0; ++ ret = REMOVE_PATH_FAILURE; ++ } ++ } ++ return (ret == REMOVE_PATH_FAILURE); + } + + int +diff --git a/multipathd/main.c b/multipathd/main.c +index 6090434c..8e2beddd 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -838,7 +838,7 @@ handle_path_wwid_change(struct path *pp, struct vectors *vecs) + return; + + udd = udev_device_ref(pp->udev); +- if (ev_remove_path(pp, vecs, 1) != 0 && pp->mpp) { ++ if (!(ev_remove_path(pp, vecs, 1) & REMOVE_PATH_SUCCESS) && pp->mpp) { + pp->dmstate = PSTATE_FAILED; + dm_fail_path(pp->mpp->alias, pp->dev_t); + } +@@ -948,8 +948,8 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map) + * Make another attempt to remove the path + */ + pp->mpp = prev_mpp; +- ret = ev_remove_path(pp, vecs, true); +- if (ret != 0) { ++ if (!(ev_remove_path(pp, vecs, true) & ++ REMOVE_PATH_SUCCESS)) { + /* + * Failure in ev_remove_path will keep + * path in pathvec in INIT_REMOVED state +@@ -960,6 +960,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map) + dm_fail_path(pp->mpp->alias, pp->dev_t); + condlog(1, "%s: failed to re-add path still mapped in %s", + pp->dev, pp->mpp->alias); ++ ret = 1; + } else if (r == PATHINFO_OK) + /* + * Path successfully freed, move on to +@@ -1167,7 +1168,6 @@ static int + uev_remove_path (struct uevent *uev, struct vectors * vecs, int need_do_map) + { + struct path *pp; +- int ret; + + condlog(3, "%s: remove path (uevent)", uev->kernel); + delete_foreign(uev->udev); +@@ -1177,21 +1177,18 @@ uev_remove_path (struct uevent *uev, struct vectors * vecs, int need_do_map) + pthread_testcancel(); + pp = find_path_by_dev(vecs->pathvec, uev->kernel); + if (pp) +- ret = ev_remove_path(pp, vecs, need_do_map); ++ ev_remove_path(pp, vecs, need_do_map); + lock_cleanup_pop(vecs->lock); +- if (!pp) { +- /* Not an error; path might have been purged earlier */ ++ if (!pp) /* Not an error; path might have been purged earlier */ + condlog(0, "%s: path already removed", uev->kernel); +- return 0; +- } +- return ret; ++ return 0; + } + + int + ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) + { + struct multipath * mpp; +- int i, retval = 0; ++ int i, retval = REMOVE_PATH_SUCCESS; + char params[PARAMS_SIZE] = {0}; + + /* +@@ -1245,7 +1242,6 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) + condlog(2, "%s: removed map after" + " removing all paths", + alias); +- retval = 0; + /* flush_map() has freed the path */ + goto out; + } +@@ -1262,11 +1258,14 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) + + if (mpp->wait_for_udev) { + mpp->wait_for_udev = 2; ++ retval = REMOVE_PATH_DELAY; + goto out; + } + +- if (!need_do_map) ++ if (!need_do_map) { ++ retval = REMOVE_PATH_DELAY; + goto out; ++ } + /* + * reload the map + */ +@@ -1275,7 +1274,7 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) + condlog(0, "%s: failed in domap for " + "removal of path %s", + mpp->alias, pp->dev); +- retval = 1; ++ retval = REMOVE_PATH_FAILURE; + } else { + /* + * update our state from kernel +@@ -1283,12 +1282,12 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) + char devt[BLK_DEV_SIZE]; + + strlcpy(devt, pp->dev_t, sizeof(devt)); ++ ++ /* setup_multipath will free the path ++ * regardless of whether it succeeds or ++ * fails */ + if (setup_multipath(vecs, mpp)) +- return 1; +- /* +- * Successful map reload without this path: +- * sync_map_state() will free it. +- */ ++ return REMOVE_PATH_MAP_ERROR; + sync_map_state(mpp); + + condlog(2, "%s: path removed from map %s", +@@ -1304,8 +1303,10 @@ out: + return retval; + + fail: ++ condlog(0, "%s: error removing path. removing map %s", pp->dev, ++ mpp->alias); + remove_map_and_stop_waiter(mpp, vecs); +- return 1; ++ return REMOVE_PATH_MAP_ERROR; + } + + static int +diff --git a/multipathd/main.h b/multipathd/main.h +index ddd953f9..bc1f938f 100644 +--- a/multipathd/main.h ++++ b/multipathd/main.h +@@ -13,6 +13,20 @@ enum daemon_status { + DAEMON_STATUS_SIZE, + }; + ++enum remove_path_result { ++ REMOVE_PATH_FAILURE = 0x0, /* path could not be removed. It is still ++ * part of the kernel map, but its state ++ * is set to INIT_REMOVED, and it will be ++ * removed at the next possible occassion */ ++ REMOVE_PATH_SUCCESS = 0x1, /* path was removed */ ++ REMOVE_PATH_DELAY = 0x2, /* path is set to be removed later. it ++ * currently still exists and is part of the ++ * kernel map */ ++ REMOVE_PATH_MAP_ERROR = 0x5, /* map was removed because of error. value ++ * includes REMOVE_PATH_SUCCESS bit ++ * because the path was also removed */ ++}; ++ + struct prout_param_descriptor; + struct prin_resp; + diff --git a/0014-multipath-free-vectors-in-configure.patch b/0014-multipath-free-vectors-in-configure.patch new file mode 100644 index 0000000..eeb09d7 --- /dev/null +++ b/0014-multipath-free-vectors-in-configure.patch @@ -0,0 +1,46 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 11 May 2021 15:25:21 -0500 +Subject: [PATCH] multipath: free vectors in configure + +configure() can retry multiple times, each time reallocing a maps and +paths vector, and leaking the previous ones. Fix this by always freeing +the vectors before configure() exits. Found by coverity. + +Signed-off-by: Benjamin Marzinski +--- + multipath/main.c | 7 ++++++- + 1 file changed, 6 insertions(+), 1 deletion(-) + +diff --git a/multipath/main.c b/multipath/main.c +index f618550d..9fe53dcd 100644 +--- a/multipath/main.c ++++ b/multipath/main.c +@@ -512,7 +512,6 @@ configure (struct config *conf, enum mpath_cmds cmd, + */ + curmp = vector_alloc(); + pathvec = vector_alloc(); +- atexit(cleanup_vecs); + + if (!curmp || !pathvec) { + condlog(0, "can not allocate memory"); +@@ -624,6 +623,11 @@ out: + if (refwwid) + FREE(refwwid); + ++ free_multipathvec(curmp, KEEP_PATHS); ++ vecs.mpvec = NULL; ++ free_pathvec(pathvec, FREE_PATHS); ++ vecs.pathvec = NULL; ++ + return r; + } + +@@ -869,6 +873,7 @@ main (int argc, char *argv[]) + conf = get_multipath_config(); + conf->retrigger_tries = 0; + conf->force_sync = 1; ++ atexit(cleanup_vecs); + while ((arg = getopt(argc, argv, ":aAdDcChl::eFfM:v:p:b:BrR:itTquUwW")) != EOF ) { + switch(arg) { + case 1: printf("optarg : %s\n",optarg); diff --git a/0015-kpartx-Don-t-leak-memory-when-getblock-returns-NULL.patch b/0015-kpartx-Don-t-leak-memory-when-getblock-returns-NULL.patch new file mode 100644 index 0000000..75004cc --- /dev/null +++ b/0015-kpartx-Don-t-leak-memory-when-getblock-returns-NULL.patch @@ -0,0 +1,27 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 11 May 2021 16:03:13 -0500 +Subject: [PATCH] kpartx: Don't leak memory when getblock returns NULL + +If a new block was allocated, but couldn't be filled, getblock will +discard it. When it does so, it needs to free the block to avoid leaking +memory. Found by coverity. + +Signed-off-by: Benjamin Marzinski +--- + kpartx/kpartx.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c +index 8ff116b8..7bc64543 100644 +--- a/kpartx/kpartx.c ++++ b/kpartx/kpartx.c +@@ -766,6 +766,8 @@ getblock (int fd, unsigned int blknr) { + if (read(fd, bp->block, secsz) != secsz) { + fprintf(stderr, "read error, sector %d\n", secnr); + blockhead = bp->next; ++ free(bp->block); ++ free(bp); + return NULL; + } + diff --git a/0016-multipathd-don-t-rescan_path-on-wwid-change-in-uev_u.patch b/0016-multipathd-don-t-rescan_path-on-wwid-change-in-uev_u.patch new file mode 100644 index 0000000..6c68ea3 --- /dev/null +++ b/0016-multipathd-don-t-rescan_path-on-wwid-change-in-uev_u.patch @@ -0,0 +1,27 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 14 May 2021 13:45:28 -0500 +Subject: [PATCH] multipathd: don't rescan_path on wwid change in + uev_update_path + +If get_uid() is returning a different wwid in uev_update_path(), then +the uid_attribute must have already gotten updated, which was the +purpose behind calling rescan_path() in the first place. + +Signed-off-by: Benjamin Marzinski +--- + multipathd/main.c | 1 - + 1 file changed, 1 deletion(-) + +diff --git a/multipathd/main.c b/multipathd/main.c +index 8e2beddd..2750f5e9 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -1359,7 +1359,6 @@ uev_update_path (struct uevent *uev, struct vectors * vecs) + condlog(0, "%s: path wwid changed from '%s' to '%s'", + uev->kernel, wwid, pp->wwid); + ev_remove_path(pp, vecs, 1); +- rescan_path(uev->udev); + needs_reinit = 1; + goto out; + } else { diff --git a/device-mapper-multipath.spec b/device-mapper-multipath.spec index 189d147..92577db 100644 --- a/device-mapper-multipath.spec +++ b/device-mapper-multipath.spec @@ -1,6 +1,6 @@ Name: device-mapper-multipath Version: 0.8.6 -Release: 2%{?dist} +Release: 3%{?dist} Summary: Tools to manage multipath devices using device-mapper License: GPLv2 URL: http://christophe.varoqui.free.fr/ @@ -20,6 +20,12 @@ Patch0007: 0007-RH-add-wwids-from-kernel-cmdline-mpath.wwids-with-A.patch Patch0008: 0008-RH-reset-default-find_mutipaths-value-to-off.patch Patch0009: 0009-RH-attempt-to-get-ANA-info-via-sysfs-first.patch Patch0010: 0010-RH-make-parse_vpd_pg83-match-scsi_id-output.patch +Patch0011: 0011-multipathd-don-t-fail-to-remove-path-once-the-map-is.patch +Patch0012: 0012-multipathd-remove-duplicate-orphan_paths-in-flush_ma.patch +Patch0013: 0013-multipathd-fix-ev_remove_path-return-code-handling.patch +Patch0014: 0014-multipath-free-vectors-in-configure.patch +Patch0015: 0015-kpartx-Don-t-leak-memory-when-getblock-returns-NULL.patch +Patch0016: 0016-multipathd-don-t-rescan_path-on-wwid-change-in-uev_u.patch # runtime Requires: %{name}-libs = %{version}-%{release} @@ -218,6 +224,16 @@ fi %{_pkgconfdir}/libdmmp.pc %changelog +* Wed May 19 2021 Benjamin Marzinski - 0.8.6-3 +- Add 0011-multipathd-don-t-fail-to-remove-path-once-the-map-is.patch +- Add 0012-multipathd-remove-duplicate-orphan_paths-in-flush_ma.patch +- Add 0013-multipathd-fix-ev_remove_path-return-code-handling.patch +- Add 0014-multipath-free-vectors-in-configure.patch +- Add 0015-kpartx-Don-t-leak-memory-when-getblock-returns-NULL.patch +- Add 0016-multipathd-don-t-rescan_path-on-wwid-change-in-uev_u.patch + * Above patches Fix bz #1938704 +- Resolves: bz #1938704 + * Wed Apr 21 2021 Benjamin Marzinski - 0.8.6-2 - Update spec file - Related: bz #1951336