From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

From: Benjamin Marzinski <bmarzins@redhat.com>

Date: Tue, 20 Nov 2018 22:05:43 -0600

Subject: [PATCH] BZ 1648397: fix mpp->hwe handling when paths are freed

Commit 1f962693 didn't deal with all of cases where a path that was part

of a multipath device could be removed. verify_paths() removes any path

that no longer exists in sysfs.  mpp->hwe needs to be updated here as

well, since verify_paths() could remove the path whose hwe vector is

pointed to by mpp->hwe.  Also, now that extract_hwe_from_path() is

called in verify_paths(), the extract_hwe_from_path() calls that

happened immediately after verify_paths() can be dropped.

The other part of this fix is mostly cosmetic. In ev_add_path(), if

domap() fails after the path is added to the multipath device and

verify_paths() is called, the code can loop back to the rescan label. If

the size of the path or the multipath device changed in the interim,

ev_add_path() would remove the path, without updating mpp->hwe; but

there is no way for the size to change. Just to make that clearer in the

code, I've moved the size check to before the rescan label so it only

happens once.

Fixes: 1f962693 "multipathd: fix mpp->hwe handling on path removal"

Cc: Martin Wilck <mwilck@suse.com>

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

---

 libmultipath/structs_vec.c |  7 +++++++

 multipathd/main.c          | 21 ++++++++-------------

 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c

index f87d69d..cd95ad4 100644

--- a/libmultipath/structs_vec.c

+++ b/libmultipath/structs_vec.c

@@ -404,6 +404,12 @@ int verify_paths(struct multipath *mpp, struct vectors *vecs)

 			vector_del_slot(mpp->paths, i);

 			i--;

+			/* Make sure mpp->hwe doesn't point to freed memory.

+			 * We call extract_hwe_from_path() below to restore

+			 * mpp->hwe

+			 */

+			if (mpp->hwe == pp->hwe)

+				mpp->hwe = NULL;

 			if ((j = find_slot(vecs->pathvec,

 					   (void *)pp)) != -1)

 				vector_del_slot(vecs->pathvec, j);

@@ -413,6 +419,7 @@ int verify_paths(struct multipath *mpp, struct vectors *vecs)

 				mpp->alias, pp->dev, pp->dev_t);

 		}

 	}

+	extract_hwe_from_path(mpp);

 	return count;

 }

diff --git a/multipathd/main.c b/multipathd/main.c

index 2d45d98..4e2835d 100644

--- a/multipathd/main.c

+++ b/multipathd/main.c

@@ -470,7 +470,6 @@ retry:

 	verify_paths(mpp, vecs);

 	mpp->action = ACT_RELOAD;

-	extract_hwe_from_path(mpp);

 	if (setup_map(mpp, params, PARAMS_SIZE, vecs)) {

 		condlog(0, "%s: failed to setup new map in update", mpp->alias);

 		retries = -1;

@@ -904,6 +903,14 @@ ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map)

 		goto fail; /* leave path added to pathvec */

 	}

 	mpp = find_mp_by_wwid(vecs->mpvec, pp->wwid);

+	if (mpp && pp->size && mpp->size != pp->size) {

+		condlog(0, "%s: failed to add new path %s, device size mismatch", mpp->alias, pp->dev);

+		int i = find_slot(vecs->pathvec, (void *)pp);

+		if (i != -1)

+			vector_del_slot(vecs->pathvec, i);

+		free_path(pp);

+		return 1;

+	}

 	if (mpp && mpp->wait_for_udev &&

 	    (pathcount(mpp, PATH_UP) > 0 ||

 	     (pathcount(mpp, PATH_GHOST) > 0 && pp->tpgs != TPGS_IMPLICIT &&

@@ -919,17 +926,6 @@ ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map)

 	pp->mpp = mpp;

 rescan:

 	if (mpp) {

-		if (pp->size && mpp->size != pp->size) {

-			condlog(0, "%s: failed to add new path %s, "

-				"device size mismatch",

-				mpp->alias, pp->dev);

-			int i = find_slot(vecs->pathvec, (void *)pp);

-			if (i != -1)

-				vector_del_slot(vecs->pathvec, i);

-			free_path(pp);

-			return 1;

-		}

-

 		condlog(4,"%s: adopting all paths for path %s",

 			mpp->alias, pp->dev);

 		if (adopt_paths(vecs->pathvec, mpp))

@@ -937,7 +933,6 @@ rescan:

 		verify_paths(mpp, vecs);

 		mpp->action = ACT_RELOAD;

-		extract_hwe_from_path(mpp);

 	} else {

 		if (!should_multipath(pp, vecs->pathvec, vecs->mpvec)) {

 			orphan_path(pp, "only one path");

-- 

2.17.2