Paths are freed in check_removed_paths() quite deeply in the call
stack. Annotate this function and some of its callers, lest we forget
about it.

Signed-off-by: Martin Wilck <[email protected]>
---
 libmultipath/structs_vec.c | 33 +++++++++++++++++++++++++++++++--
 multipathd/main.c          | 10 ++++++----
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 62ee450..9905ff2 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -563,6 +563,34 @@ static struct path *find_devt_in_pathgroups(const struct 
multipath *mpp,
        return NULL;
 }
 
+/*
+ * check_removed_paths()
+ *
+ * This function removes paths from the pathvec and frees them if they have
+ * been marked for removal (INIT_REMOVED, INIT_PARTIAL).
+ * This is important because some callers (e.g. uev_add_path->ev_remove_path())
+ * rely on the paths being actually gone when the call stack returns.
+ * Be sure not to call it while these paths are still referenced elsewhere
+ * (e.g. from coalesce_paths(), where curmp may still reference them).
+ *
+ * Most important call stacks in multipath-tools 0.13.0:
+ *
+ * checker_finished()
+ *   sync_mpp()
+ *     do_sync_mpp()
+ *       update_multipath_strings()
+ *         sync_paths()
+ *           check_removed_paths()
+ *
+ * [multiple callers including update_map(), ev_remove_path(), ...]
+ *   setup_multipath()
+ *     refresh_multipath()
+ *       update_multipath_strings()
+ *         sync_paths()
+ *           check_removed_paths()
+ *
+ * refresh_multipath() is also called from a couple of CLI handlers.
+ */
 static void check_removed_paths(const struct multipath *mpp, vector pathvec)
 {
        struct path *pp;
@@ -583,6 +611,7 @@ static void check_removed_paths(const struct multipath 
*mpp, vector pathvec)
        }
 }
 
+/* This function may free paths. See check_removed_paths(). */
 void sync_paths(struct multipath *mpp, vector pathvec)
 {
        struct path *pp;
@@ -611,8 +640,8 @@ void sync_paths(struct multipath *mpp, vector pathvec)
                pp->mpp = mpp;
 }
 
-int
-update_multipath_strings(struct multipath *mpp, vector pathvec)
+/* This function may free paths. See check_removed_paths(). */
+int update_multipath_strings(struct multipath *mpp, vector pathvec)
 {
        struct pathgroup *pgp;
        int i, r = DMP_ERR;
diff --git a/multipathd/main.c b/multipathd/main.c
index 7b522e8..697e269 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -503,6 +503,7 @@ remove_maps_and_stop_waiters(struct vectors *vecs)
        remove_maps(vecs);
 }
 
+/* This function may free paths. See check_removed_paths(). */
 int refresh_multipath(struct vectors *vecs, struct multipath *mpp)
 {
        if (update_multipath_strings(mpp, vecs->pathvec) != DMP_OK) {
@@ -513,6 +514,7 @@ int refresh_multipath(struct vectors *vecs, struct 
multipath *mpp)
        return 0;
 }
 
+/* This function may free paths. See check_removed_paths(). */
 int setup_multipath(struct vectors *vecs, struct multipath *mpp)
 {
        if (refresh_multipath(vecs, mpp) != 0)
@@ -2519,8 +2521,8 @@ get_new_state(struct path *pp)
        return newstate;
 }
 
-static int
-do_sync_mpp(struct vectors * vecs, struct multipath *mpp)
+/* This function may free paths. See check_removed_paths(). */
+static int do_sync_mpp(struct vectors *vecs, struct multipath *mpp)
 {
        int i, ret;
        struct path *pp;
@@ -2538,8 +2540,8 @@ do_sync_mpp(struct vectors * vecs, struct multipath *mpp)
        return ret;
 }
 
-static int
-sync_mpp(struct vectors * vecs, struct multipath *mpp, unsigned int ticks)
+/* This function may free paths. See check_removed_paths(). */
+static int sync_mpp(struct vectors *vecs, struct multipath *mpp, unsigned int 
ticks)
 {
        if (mpp->sync_tick)
                mpp->sync_tick -= (mpp->sync_tick > ticks) ? ticks :
-- 
2.52.0


Reply via email to