autofs-5.1.7 - fix inconsistent locking in umount_subtree_mounts()

Some map entry cache locking inconsistencies have crept in.

In umount_subtree_mounts() the cache write lock should be held when
deleting multi-mount cache entries.

Signed-off-by: Ian Kent <raven@themaw.net>

3 files changed, 31 insertions, 20 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 1dded118..64e619ec 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -15,6 +15,7 @@
 - eliminate clean_stale_multi_triggers().
 - simplify mount_subtree() mount check.
 - fix mnts_get_expire_list() expire list construction.
+- fix inconsistent locking in umount_subtree_mounts().
 
 25/01/2021 autofs-5.1.7
 - make bind mounts propagation slave by default.
diff --git a/daemon/automount.c b/daemon/automount.c
index 7fa92877..93bd8556 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -527,8 +527,11 @@ static int umount_subtree_mounts(struct autofs_point *ap, const char *path, unsi
 	struct mapent_cache *mc;
 	struct mapent *me;
 	unsigned int is_mm_root = 0;
+	int cur_state;
 	int left;
 
+	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cur_state);
+
 	me = lookup_source_mapent(ap, path, LKP_DISTINCT);
 	if (!me) {
 		char *ind_key;
@@ -548,11 +551,11 @@ static int umount_subtree_mounts(struct autofs_point *ap, const char *path, unsi
 	left = 0;
 
 	if (me && me->multi) {
-		char root[PATH_MAX];
+		char root[PATH_MAX + 1];
+		char key[PATH_MAX + 1];
+		struct mapent *tmp;
+		int status;
 		char *base;
-		int cur_state;
-
-		pthread_cleanup_push(cache_lock_cleanup, mc);
 
 		if (!strchr(me->multi->key, '/'))
 			/* Indirect multi-mount root */
@@ -567,25 +570,40 @@ static int umount_subtree_mounts(struct autofs_point *ap, const char *path, unsi
 		else
 			base = me->key + strlen(root);
 
-		pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cur_state);
-		/* Lock the closest parent nesting point for umount */
-		cache_multi_writelock(me->parent);
-		if (umount_multi_triggers(ap, me, root, base)) {
+		left = umount_multi_triggers(ap, me, root, base);
+		if (left) {
 			warn(ap->logopt,
 			     "some offset mounts still present under %s", path);
+		}
+
+		strcpy(key, me->key);
+
+		cache_unlock(mc);
+		cache_writelock(mc);
+		tmp = cache_lookup_distinct(mc, key);
+		/* mapent went away while we waited? */
+		if (tmp != me) {
+			cache_unlock(mc);
+			pthread_setcancelstate(cur_state, NULL);
+			return 0;
+		}
+
+		if (!left && is_mm_root) {
+			status = cache_delete_offset_list(mc, me->key);
+			if (status != CHE_OK)
+				warn(ap->logopt, "couldn't delete offset list");
 			left++;
 		}
-		cache_multi_unlock(me->parent);
+
 		if (ap->entry->maps &&
 		    (ap->entry->maps->flags & MAP_FLAG_FORMAT_AMD))
 			cache_pop_mapent(me);
-		pthread_setcancelstate(cur_state, NULL);
-		pthread_cleanup_pop(0);
 	}
-
 	if (me)
 		cache_unlock(mc);
 
+	pthread_setcancelstate(cur_state, NULL);
+
 	if (left || is_autofs_fs)
 		return left;
 
diff --git a/lib/mounts.c b/lib/mounts.c
index 87813b16..5ebfe5fd 100644
--- a/lib/mounts.c
+++ b/lib/mounts.c
@@ -2736,9 +2736,6 @@ int umount_multi_triggers(struct autofs_point *ap, struct mapent *me, char *root
 	left = do_umount_multi_triggers(ap, me, root, base);
 
 	if (!left && me->multi == me) {
-		struct mapent_cache *mc = me->mc;
-		int status;
-
 		/*
 		 * Special case.
 		 * If we can't umount the root container then we can't
@@ -2756,11 +2753,6 @@ int umount_multi_triggers(struct autofs_point *ap, struct mapent *me, char *root
 			}
 		}
 
-		/* We're done - clean out the offsets */
-		status = cache_delete_offset_list(mc, me->key);
-		if (status != CHE_OK)
-			warn(ap->logopt, "couldn't delete offset list");
-
 	       /* check for mounted mount entry and remove it if found */
                mnts_remove_mount(root, MNTS_MOUNTED);
 	}