On Sat, 2021-11-27 at 18:15 +0800, lixiaokeng wrote:
> 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@xxxxxxxxxx>
> ---
> 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);
> + }
> +
No, we can't do this. It would invalidate all the work we did for
INIT_REMOVED. It's the very idea of INIT_REMOVED that we do NOT
immediately clear the path from our data structures. The main problem
is that INIT_REMOVED paths may still be part of the map in the kernel.
Thus when we re-read the kernel state, the path will be re-added to
mpp->paths.
We need to rework the free_multipath() logic. It might be sufficient
to simply clear the references to the devices in mpp before calling
orphan_paths(). Can you try the attached patch, please?
Regards
Martin
From 03fa4c40289658cb8b70c8fa8c265f2d07dee42d Mon Sep 17 00:00:00 2001
From: Martin Wilck <mwilck@xxxxxxxx>
Date: Mon, 29 Nov 2021 00:16:07 +0100
Subject: [PATCH] libmultipath: remove_map(): make sure orphaned paths aren't
referenced
... by the paths and pg vectors of the map to be removed.
Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
---
libmultipath/structs_vec.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 42c06a8..105ee20 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -341,6 +341,10 @@ remove_map(struct multipath *mpp, vector pathvec, vector mpvec)
{
int i;
+ free_pathvec(mpp->paths, KEEP_PATHS);
+ free_pgvec(mpp->pg, KEEP_PATHS);
+ mpp->paths = mpp->pg = NULL;
+
/*
* clear references to this map
*/
--
2.33.1
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
