[dm-devel] [PATCH] libmultipath: clear removed path from mpp

[lixiaokeng]

multipathd[3525635]: ==3525635==ERROR: AddressSanitizer: heap-use-after-free on address 0xffffa4902fc0 at pc 0xffffac7d5b88 bp 0xffffa948dac0 sp 0xffffa948dae0
multipathd[3525635]: READ of size 8 at 0xffffa4902fc0 thread T7
multipathd[3525635]:    #0 0xffffac7d5b87 in free_multipath (/usr/lib64/libmultipath.so.0+0x4bb87)
multipathd[3525635]:    #1 0xaaaad6cf7057  (/usr/sbin/multipathd+0x17057)
multipathd[3525635]:    #2 0xaaaad6cf78eb  (/usr/sbin/multipathd+0x178eb)
multipathd[3525635]:    #3 0xaaaad6cff4df  (/usr/sbin/multipathd+0x1f4df)
multipathd[3525635]:    #4 0xaaaad6cfffe7  (/usr/sbin/multipathd+0x1ffe7)
multipathd[3525635]:    #5 0xffffac807be3 in uevent_dispatch (/usr/lib64/libmultipath.so.0+0x7dbe3)
multipathd[3525635]:    #6 0xaaaad6cf563f  (/usr/sbin/multipathd+0x1563f)
multipathd[3525635]:    #7 0xffffac6877af  (/usr/lib64/libpthread.so.0+0x87af)
multipathd[3525635]:    #8 0xffffac44118b  (/usr/lib64/libc.so.6+0xd518b)
multipathd[3525635]: 0xffffa4902fc0 is located 1344 bytes inside of 1440-byte region [0xffffa4902a80,0xffffa4903020)
multipathd[3525635]: freed by thread T7 here:
multipathd[3525635]:    #0 0xffffac97d703 in free (/usr/lib64/libasan.so.4+0xd0703)
multipathd[3525635]:    #1 0xffffac824827 in orphan_paths (/usr/lib64/libmultipath.so.0+0x9a827)
multipathd[3525635]:    #2 0xffffac824a43 in remove_map (/usr/lib64/libmultipath.so.0+0x9aa43)
multipathd[3525635]:    #3 0xaaaad6cf7057  (/usr/sbin/multipathd+0x17057)
multipathd[3525635]:    #4 0xaaaad6cf78eb  (/usr/sbin/multipathd+0x178eb)
multipathd[3525635]:    #5 0xaaaad6cff4df  (/usr/sbin/multipathd+0x1f4df)
multipathd[3525635]:    #6 0xaaaad6cfffe7  (/usr/sbin/multipathd+0x1ffe7)
multipathd[3525635]:    #7 0xffffac807be3 in uevent_dispatch (/usr/lib64/libmultipath.so.0+0x7dbe3)
multipathd[3525635]:    #8 0xaaaad6cf563f  (/usr/sbin/multipathd+0x1563f)
multipathd[3525635]:    #9 0xffffac6877af  (/usr/lib64/libpthread.so.0+0x87af)
multipathd[3525635]:    #10 0xffffac44118b  (/usr/lib64/libc.so.6+0xd518b)

When mpp only has one path and log out the path, there is an asan error.
In remove_mpp, the pp is freed firstly in orphan_path but is accessed,
changed in free_multipath later. Before free_path(pp), the pp should be
cleared from pp->mpp.

When pp is set to REMOVED， it will be an orphan path. At the same time,
clear it from it's map.

Signed-off-by: Lixiaokeng <lixiaokeng at huawei.com>
---
 libmultipath/structs_vec.c | 20 +++++++++++++++++---
 multipathd/main.c          |  8 --------
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 85d97ac1..ab5311cc 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -318,7 +318,9 @@ void orphan_paths(vector pathvec, struct multipath *mpp, const char *reason)

 void set_path_removed(struct path *pp)
 {
+	int i, j;
 	struct multipath *mpp = pp->mpp;
+	struct pathgroup * pgp;

 	orphan_path(pp, "removed");
 	/*
@@ -329,6 +331,20 @@ void set_path_removed(struct path *pp)
 		condlog(0, "%s: internal error: mpp == NULL", pp->dev);
 		return;
 	}
+
+	/*
+	 * The path is removed, clear it from mp->paths and mpp->pgs.
+	 */
+	i = find_slot(mpp->paths, pp);
+	if (i != -1)
+		vector_del_slot(mpp->paths, i);
+
+	vector_foreach_slot(mpp->pg, pgp, j) {
+		i = find_slot(pgp->paths, (void *)pp);
+		if (i != -1)
+			vector_del_slot(pgp->paths, i);
+	}
+
 	pp->mpp = mpp;
 	pp->initialized = INIT_REMOVED;
 }
@@ -731,15 +747,13 @@ int verify_paths(struct multipath *mpp)
 					mpp->alias, pp->dev);
 			}
 			count++;
-			vector_del_slot(mpp->paths, i);
-			i--;
-
 			/*
 			 * Don't delete path from pathvec yet. We'll do this
 			 * after the path has been removed from the map, in
 			 * sync_paths().
 			 */
 			set_path_removed(pp);
+			i--;
 		} else {
 			condlog(4, "%s: verified path %s dev_t %s",
 				mpp->alias, pp->dev, pp->dev_t);
diff --git a/multipathd/main.c b/multipathd/main.c
index 3f675136..388207a2 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1226,14 +1226,6 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map)
 			goto fail;
 		}

-		/*
-		 * we have to explicitly remove pp from mpp->paths,
-		 * update_mpp_paths() doesn't do that.
-		 */
-		i = find_slot(mpp->paths, pp);
-		if (i != -1)
-			vector_del_slot(mpp->paths, i);
-
 		/*
 		 * remove the map IF removing the last path
 		 */
--