autofs-5.1.7 - eliminate clean_stale_multi_triggers()

Eliminate clean_stale_multi_triggers() by checking for stale offsets at
the time mount_subtree() is called.

This should result in the same behaviour but eliminate an additional
seperate traversal of the offset list.

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

3 files changed, 43 insertions, 177 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 5a3bedc1..b1ce7b69 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -12,6 +12,7 @@
 - remove redundant variables from mount_autofs_offset().
 - remove unused parameter form do_mount_autofs_offset().
 - refactor umount_multi_triggers().
+- eliminate clean_stale_multi_triggers().
 
 25/01/2021 autofs-5.1.7
 - make bind mounts propagation slave by default.
diff --git a/lib/mounts.c b/lib/mounts.c
index 5268ba5b..a9abbebf 100644
--- a/lib/mounts.c
+++ b/lib/mounts.c
@@ -2601,10 +2601,44 @@ static int do_umount_offset(struct autofs_point *ap, struct mapent *oe, const ch
 	oe_base = oe->key + strlen(root);
 	left += do_umount_multi_triggers(ap, oe, root, oe_base);
 
+	/*
+	 * If an offset that has an active mount has been removed
+	 * from the multi-mount we don't want to attempt to trigger
+	 * mounts for it. Obviously this is because it has been
+	 * removed, but less obvious is the potential strange
+	 * behaviour that can result if we do try and mount it
+	 * again after it's been expired. For example, if an NFS
+	 * file system is no longer exported and is later umounted
+	 * it can be mounted again without any error message but
+	 * shows as an empty directory. That's going to confuse
+	 * people for sure.
+	 *
+	 * If the mount cannot be umounted (the process is now
+	 * using a stale mount) the offset needs to be invalidated
+	 * so no further mounts will be attempted but the offset
+	 * cache entry must remain so expires can continue to
+	 * attempt to umount it. If the mount can be umounted and
+	 * the offset is removed, at least for NFS we will get
+	 * ESTALE errors when attempting list the directory.
+	 */
 	if (oe->ioctlfd != -1 ||
 	    is_mounted(oe->key, MNTS_REAL)) {
-		left++;
-		return left;
+		if (umount_ent(ap, oe->key) &&
+		    is_mounted(oe->key, MNTS_REAL)) {
+			debug(ap->logopt,
+			      "offset %s has active mount, invalidate",
+			      oe->key);
+			/*
+			 * Ok, so we shouldn't modify the mapent but
+			 * mount requests are blocked at a point above
+			 * this and expire only uses the mapent key.
+			 */
+			if (oe->mapent) {
+				free(oe->mapent);
+				oe->mapent = NULL;
+			}
+			return ++left;
+		}
 	}
 
 	debug(ap->logopt, "umount offset %s", oe->key);
@@ -2666,6 +2700,11 @@ int mount_multi_triggers(struct autofs_point *ap, struct mapent *me,
 		oe = cache_lookup_distinct(me->mc, key);
 		if (!oe || !oe->mapent)
 			goto cont;
+		if (oe->age != me->multi->age) {
+			/* Best effort */
+			do_umount_offset(ap, oe, root);
+			goto cont;
+		}
 
 		mounted += do_mount_autofs_offset(ap, oe, root);
 
@@ -2725,169 +2764,3 @@ int umount_multi_triggers(struct autofs_point *ap, struct mapent *me, char *root
 
 	return left;
 }
-
-int clean_stale_multi_triggers(struct autofs_point *ap,
-			       struct mapent *me, char *top, const char *base)
-{
-	char *root;
-	char mm_top[PATH_MAX + 1];
-	char path[PATH_MAX + 1];
-	char *offset;
-	struct mapent *oe;
-	struct list_head *mm_root, *pos;
-	const char o_root[] = "/";
-	const char *mm_base;
-	int left, start;
-	unsigned int root_len;
-	unsigned int mm_base_len;
-	time_t age;
-
-	if (top)
-		root = top;
-	else {
-		if (!strchr(me->multi->key, '/'))
-			/* Indirect multi-mount root */
-			/* sprintf okay - if it's mounted, it's
-			 * PATH_MAX or less bytes */
-			sprintf(mm_top, "%s/%s", ap->path, me->multi->key);
-		else
-			strcpy(mm_top, me->multi->key);
-		root = mm_top;
-	}
-
-	left = 0;
-	start = strlen(root);
-
-	mm_root = &me->multi->multi_list;
-
-	if (!base)
-		mm_base = o_root;
-	else
-		mm_base = base;
-
-	pos = NULL;
-	offset = path;
-	root_len = start;
-	mm_base_len = strlen(mm_base);
-	age = me->multi->age;
-
-	while ((offset = cache_get_offset(mm_base, offset, start, mm_root, &pos))) {
-		char key[PATH_MAX + 1];
-		int key_len = root_len + strlen(offset);
-		char *oe_base;
-		int ret;
-
-		if (mm_base_len > 1)
-			key_len += mm_base_len;
-
-		if (key_len > PATH_MAX) {
-			warn(ap->logopt, "path loo long");
-			continue;
-		}
-
-		strcpy(key, root);
-		if (mm_base_len > 1)
-			strcat(key, mm_base);
-		strcat(key, offset);
-
-		oe = cache_lookup_distinct(me->mc, key);
-		/* root offset is a special case */
-		if (!oe || (strlen(oe->key) - start) == 1)
-			continue;
-
-		/* Check for and umount stale subtree offsets */
-		oe_base = oe->key + strlen(root);
-		ret = clean_stale_multi_triggers(ap, oe, root, oe_base);
-		left += ret;
-		if (ret)
-			continue;
-
-		if (oe->age == age)
-			continue;
-
-		/*
-		 * If an offset that has an active mount has been removed
-		 * from the multi-mount we don't want to attempt to trigger
-		 * mounts for it. Obviously this is because it has been
-		 * removed, but less obvious is the potential strange
-		 * behaviour that can result if we do try and mount it
-		 * again after it's been expired. For example, if an NFS
-		 * file system is no longer exported and is later umounted
-		 * it can be mounted again without any error message but
-		 * shows as an empty directory. That's going to confuse
-		 * people for sure.
-		 *
-		 * If the mount cannot be umounted (the process is now
-		 * using a stale mount) the offset needs to be invalidated
-		 * so no further mounts will be attempted but the offset
-		 * cache entry must remain so expires can continue to
-		 * attempt to umount it. If the mount can be umounted and
-		 * the offset is removed, at least for NFS we will get
-		 * ESTALE errors when attempting list the directory.
-		 */
-		if (oe->ioctlfd != -1 ||
-		    is_mounted(oe->key, MNTS_REAL)) {
-			if (umount_ent(ap, oe->key) &&
-			    is_mounted(oe->key, MNTS_REAL)) {
-				debug(ap->logopt,
-				      "offset %s has active mount, invalidate",
-				      oe->key);
-				if (oe->mapent) {
-					free(oe->mapent);
-					oe->mapent = NULL;
-				}
-				left++;
-				continue;
-			}
-		}
-
-		debug(ap->logopt, "umount offset %s", oe->key);
-
-		if (umount_autofs_offset(ap, oe)) {
-			warn(ap->logopt, "failed to umount offset %s", key);
-			left++;
-		} else {
-			struct stat st;
-
-			/* Mount point not ours to delete ? */
-			if (!(oe->flags & MOUNT_FLAG_DIR_CREATED)) {
-				debug(ap->logopt, "delete offset key %s", key);
-				if (cache_delete_offset(oe->mc, key) == CHE_FAIL)
-					error(ap->logopt,
-					     "failed to delete offset key %s", key);
-				continue;
-			}
-
-			/*
-			 * An error due to partial directory removal is
-			 * ok so only try and remount the offset if the
-			 * actual mount point still exists.
-			 */
-			ret = rmdir_path_offset(ap, oe);
-			if (ret == -1 && !stat(oe->key, &st)) {
-				ret = do_mount_autofs_offset(ap, oe, root);
-				if (ret) {
-					left++;
-					/* But we did origianlly create this */
-					oe->flags |= MOUNT_FLAG_DIR_CREATED;
-					continue;
-				}
-				/*
-				 * Fall through if the trigger can't be mounted
-				 * again, since there is no offset there can't
-				 * be any mount requests so remove the map
-				 * entry from the cache. There's now a dead
-				 * offset mount, but what else can we do ....
-				 */
-			}
-
-			debug(ap->logopt, "delete offset key %s", key);
-
-			if (cache_delete_offset(oe->mc, key) == CHE_FAIL)
-				error(ap->logopt,
-				     "failed to delete offset key %s", key);
-		}
-	}
-
-	return left;
-}
diff --git a/modules/parse_sun.c b/modules/parse_sun.c
index f42af7b7..f4d5125c 100644
--- a/modules/parse_sun.c
+++ b/modules/parse_sun.c
@@ -1176,7 +1176,7 @@ static int mount_subtree(struct autofs_point *ap, struct mapent *me,
 
 		/* Mount root offset if it exists */
 		ro = cache_lookup_distinct(me->mc, key);
-		if (ro) {
+		if (ro && ro->age == me->multi->age) {
 			char *myoptions, *ro_loc;
 			int namelen = name ? strlen(name) : 0;
 			int ro_len;
@@ -1610,14 +1610,6 @@ dont_expand:
 			free(myoptions);
 		} while (*p == '/' || (*p == '"' && *(p + 1) == '/'));
 
-		/*
-		 * We've got the ordered list of multi-mount entries so go
-		 * through and remove any stale entries if this is the top
-		 * of the multi-mount and set the parent entry of each.
-		 */
-		if (me == me->multi)
-			clean_stale_multi_triggers(ap, me, NULL, NULL);
-
 		rv = mount_subtree(ap, me, name, NULL, options, ctxt);
 
 		cache_multi_unlock(me);