autofs-5.1.8 - fix handling of incorrect return from umount_ent()

Commit 0210535df4b ("autofs-5.1.0 - gaurd against incorrect umount
return") guards against umount_ent() returning a fail when the mount
has actually been umounted.

But we also see umount_ent() return success when in fact the mount has
not been umounted leading to incorrect handling of automounts.

So checking the return of umount_ent() isn't always giving the correct
result in more than just one case, consequently we should ignore the
result from the spawned umount(8) and check if the mount has in fact
been umounted.

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

3 files changed, 12 insertions, 11 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 5402b88d..72a5aa59 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -15,6 +15,7 @@
 - avoid calling pthread_getspecific() with NULL key_thread_attempt_id.
 - fix sysconf(3) return handling.
 - remove nonstrict parameter from tree_mapent_umount_offsets().
+- fix handling of incorrect return from umount_ent().
 
 19/10/2021 autofs-5.1.8
 - add xdr_exports().
diff --git a/daemon/automount.c b/daemon/automount.c
index 353e4f54..85847edf 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -609,8 +609,7 @@ static int umount_subtree_mounts(struct autofs_point *ap, const char *path, unsi
 		struct mnt_list *mnt;
 
 		debug(ap->logopt, "unmounting dir = %s", path);
-		if (umount_ent(ap, path) &&
-		    is_mounted(path, MNTS_REAL)) {
+		if (umount_ent(ap, path)) {
 			warn(ap->logopt, "could not umount dir %s", path);
 			left++;
 			goto done;
diff --git a/lib/mounts.c b/lib/mounts.c
index 617c1d54..a3f9dfd7 100644
--- a/lib/mounts.c
+++ b/lib/mounts.c
@@ -1869,8 +1869,7 @@ static int tree_mapent_umount_offset(struct mapent *oe, void *ptr)
 	 */
 	if (oe->ioctlfd != -1 ||
 	    is_mounted(oe->key, MNTS_REAL)) {
-		if (umount_ent(ap, oe->key) &&
-		    is_mounted(oe->key, MNTS_REAL)) {
+		if (umount_ent(ap, oe->key)) {
 			debug(ap->logopt,
 			      "offset %s has active mount, invalidate",
 			      oe->key);
@@ -2010,8 +2009,7 @@ int tree_mapent_umount_offsets(struct mapent *oe)
 		 */
 		if (is_mounted(mp, MNTS_REAL)) {
 			info(ap->logopt, "unmounting dir = %s", mp);
-			if (umount_ent(ap, mp) &&
-			    is_mounted(mp, MNTS_REAL)) {
+			if (umount_ent(ap, mp)) {
 				if (!tree_mapent_mount_offsets(oe, 1))
 					warn(ap->logopt,
 					     "failed to remount offset triggers");
@@ -2982,6 +2980,7 @@ void set_direct_mount_tree_catatonic(struct autofs_point *ap, struct mapent *me)
 
 int umount_ent(struct autofs_point *ap, const char *path)
 {
+	unsigned int mounted;
 	int rv;
 
 	if (ap->state != ST_SHUTDOWN_FORCE)
@@ -2993,6 +2992,8 @@ int umount_ent(struct autofs_point *ap, const char *path)
 		rv = spawn_umount(ap->logopt, "-l", path, NULL);
 	}
 
+	mounted = is_mounted(path, MNTS_REAL);
+
 	if (rv && (ap->state == ST_SHUTDOWN_FORCE || ap->state == ST_SHUTDOWN)) {
 		/*
 		 * Verify that we actually unmounted the thing.  This is a
@@ -3004,20 +3005,20 @@ int umount_ent(struct autofs_point *ap, const char *path)
 		 * so that we do not try to call rmdir_path on the
 		 * directory.
 		 */
-		if (is_mounted(path, MNTS_REAL)) {
+		if (mounted) {
 			crit(ap->logopt,
 			     "the umount binary reported that %s was "
 			     "unmounted, but there is still something "
 			     "mounted on this path.", path);
-			rv = -1;
+			mounted = -1;
 		}
 	}
 
-	/* On success, check for mounted mount and remove it if found */
-	if (!rv)
+	/* If mount is gone remove it from mounted mounts list. */
+	if (!mounted)
 		mnts_remove_mount(path, MNTS_MOUNTED);
 
-	return rv;
+	return mounted;
 }
 
 int umount_amd_ext_mount(struct autofs_point *ap, const char *path)