c64a48b95f
Pull in latest upstream post-tag commits
* Patches 0001-0015 are from
https://github.com/openSUSE/multipath-tools/tree/queue and are
already queued for upstream
Rename files
* Previous patches 0001-0010 and now patches 0016-0025
227 lines
7.1 KiB
Diff
227 lines
7.1 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Benjamin Marzinski <bmarzins@redhat.com>
|
|
Date: Mon, 17 May 2021 11:29:56 -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 <bmarzins@redhat.com>
|
|
Reviewed-by: Martin Wilck <mwilck@suse.com>
|
|
---
|
|
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 266d6b44..26a4e44e 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;
|
|
|