Blob Blame History Raw
autofs-5.1.6 - make bind mounts propagation slave by default

From: Ian Kent <raven@themaw.net>

Make setting mount propagation on bind mounts mandatory with a default
of propagation slave.

When using multi-mounts that have bind mounts the bind mount will have
the same properties as its parent which is commonly propagation shared.
And if the mount target is also propagation shared this can lead to a
deadlock when attempting to access the offset mounts. When this happens
an unwanted offset mount is propagated back to the target file system
resulting in a deadlock since the automount target is itself an
(unwanted) automount trigger.

This problem has been present much longer than I originally thought,
perhaps since mount propagation was introduced into the kernel, so
explicitly setting bind mount propagation is the sensible thing to do.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 CHANGELOG            |    1 +
 include/automount.h  |    9 +++++----
 lib/master_parse.y   |   29 +++++++++++++++++------------
 lib/master_tok.l     |    1 +
 modules/mount_bind.c |   40 ++++++++++++++++++++++------------------
 5 files changed, 46 insertions(+), 34 deletions(-)

--- autofs-5.1.4.orig/include/automount.h
+++ autofs-5.1.4/include/automount.h
@@ -551,14 +551,15 @@ struct kernel_mod_version {
 #define MOUNT_FLAG_AMD_CACHE_ALL	0x0080
 
 /* Set mount propagation for bind mounts */
-#define MOUNT_FLAG_SLAVE		0x0100
-#define MOUNT_FLAG_PRIVATE		0x0200
+#define MOUNT_FLAG_SHARED		0x0100
+#define MOUNT_FLAG_SLAVE		0x0200
+#define MOUNT_FLAG_PRIVATE		0x0400
 
 /* Use strict expire semantics if requested and kernel supports it */
-#define MOUNT_FLAG_STRICTEXPIRE		0x0400
+#define MOUNT_FLAG_STRICTEXPIRE		0x0800
 
 /* Indicator for applications to ignore the mount entry */
-#define MOUNT_FLAG_IGNORE		0x0800
+#define MOUNT_FLAG_IGNORE		0x1000
 
 struct autofs_point {
 	pthread_t thid;
--- autofs-5.1.4.orig/lib/master_parse.y
+++ autofs-5.1.4/lib/master_parse.y
@@ -59,8 +59,6 @@ static long timeout;
 static long negative_timeout;
 static unsigned symlnk;
 static unsigned strictexpire;
-static unsigned slave;
-static unsigned private;
 static unsigned nobind;
 static unsigned ghost;
 extern unsigned global_selection_options;
@@ -72,6 +70,14 @@ static int tmp_argc;
 static char **local_argv;
 static int local_argc;
 
+#define PROPAGATION_SHARED	MOUNT_FLAG_SHARED
+#define PROPAGATION_SLAVE	MOUNT_FLAG_SLAVE
+#define PROPAGATION_PRIVATE	MOUNT_FLAG_PRIVATE
+#define PROPAGATION_MASK	(MOUNT_FLAG_SHARED | \
+				 MOUNT_FLAG_SLAVE  | \
+				 MOUNT_FLAG_PRIVATE)
+static unsigned int propagation;
+
 static char errstr[MAX_ERR_LEN];
 
 static unsigned int verbose;
@@ -106,7 +112,7 @@ static int master_fprintf(FILE *, char *
 %token MAP
 %token OPT_TIMEOUT OPT_NTIMEOUT OPT_NOBIND OPT_NOGHOST OPT_GHOST OPT_VERBOSE
 %token OPT_DEBUG OPT_RANDOM OPT_USE_WEIGHT OPT_SYMLINK OPT_MODE
-%token OPT_STRICTEXPIRE OPT_SLAVE OPT_PRIVATE
+%token OPT_STRICTEXPIRE OPT_SHARED OPT_SLAVE OPT_PRIVATE
 %token COLON COMMA NL DDASH
 %type <strtype> map
 %type <strtype> options
@@ -208,6 +214,7 @@ line:
 	| PATH OPT_TIMEOUT { master_notify($1); YYABORT; }
 	| PATH OPT_SYMLINK { master_notify($1); YYABORT; }
 	| PATH OPT_STRICTEXPIRE { master_notify($1); YYABORT; }
+	| PATH OPT_SHARED { master_notify($1); YYABORT; }
 	| PATH OPT_SLAVE { master_notify($1); YYABORT; }
 	| PATH OPT_PRIVATE { master_notify($1); YYABORT; }
 	| PATH OPT_NOBIND { master_notify($1); YYABORT; }
@@ -622,8 +629,9 @@ daemon_option: OPT_TIMEOUT NUMBER { time
 	| OPT_NTIMEOUT NUMBER { negative_timeout = $2; }
 	| OPT_SYMLINK	{ symlnk = 1; }
 	| OPT_STRICTEXPIRE { strictexpire = 1; }
-	| OPT_SLAVE	{ slave = 1; }
-	| OPT_PRIVATE	{ private = 1; }
+	| OPT_SHARED	{ propagation = PROPAGATION_SHARED; }
+	| OPT_SLAVE	{ propagation = PROPAGATION_SLAVE; }
+	| OPT_PRIVATE	{ propagation = PROPAGATION_PRIVATE; }
 	| OPT_NOBIND	{ nobind = 1; }
 	| OPT_NOGHOST	{ ghost = 0; }
 	| OPT_GHOST	{ ghost = 1; }
@@ -697,8 +705,7 @@ static void local_init_vars(void)
 	negative_timeout = 0;
 	symlnk = 0;
 	strictexpire = 0;
-	slave = 0;
-	private = 0;
+	propagation = PROPAGATION_SLAVE;
 	nobind = 0;
 	ghost = defaults_get_browse_mode();
 	random_selection = global_selection_options & MOUNT_FLAG_RANDOM_SELECT;
@@ -888,7 +895,6 @@ int master_parse_entry(const char *buffe
 			ghost = 1;
 	}
 
-
 	if (!entry->ap) {
 		ret = master_add_autofs_point(entry, logopt, nobind, ghost, 0);
 		if (!ret) {
@@ -899,6 +905,9 @@ int master_parse_entry(const char *buffe
 			return 0;
 		}
 	}
+	entry->ap->flags &= ~(PROPAGATION_MASK);
+	entry->ap->flags |= propagation;
+
 	if (random_selection)
 		entry->ap->flags |= MOUNT_FLAG_RANDOM_SELECT;
 	if (use_weight)
@@ -907,10 +916,6 @@ int master_parse_entry(const char *buffe
 		entry->ap->flags |= MOUNT_FLAG_SYMLINK;
 	if (strictexpire)
 		entry->ap->flags |= MOUNT_FLAG_STRICTEXPIRE;
-	if (slave)
-		entry->ap->flags |= MOUNT_FLAG_SLAVE;
-	if (private)
-		entry->ap->flags |= MOUNT_FLAG_PRIVATE;
 	if (negative_timeout)
 		entry->ap->negative_timeout = negative_timeout;
 	if (mode && mode < LONG_MAX)
--- autofs-5.1.4.orig/lib/master_tok.l
+++ autofs-5.1.4/lib/master_tok.l
@@ -389,6 +389,7 @@ MODE		(--mode{OPTWS}|--mode{OPTWS}={OPTW
 	-?symlink		{ return(OPT_SYMLINK); }
 	-?nobind		{ return(OPT_NOBIND); }
 	-?nobrowse		{ return(OPT_NOGHOST); }
+	-?shared		{ return(OPT_SHARED); }
 	-?slave			{ return(OPT_SLAVE); }
 	-?private		{ return(OPT_PRIVATE); }
 	-?strictexpire		{ return(OPT_STRICTEXPIRE); }
--- autofs-5.1.4.orig/modules/mount_bind.c
+++ autofs-5.1.4/modules/mount_bind.c
@@ -153,6 +153,7 @@ int mount_mount(struct autofs_point *ap,
 
 	if (!symlnk && bind_works) {
 		int status, existed = 1;
+		int flags;
 
 		debug(ap->logopt, MODPREFIX "calling mkdir_path %s", fullpath);
 
@@ -190,24 +191,27 @@ int mount_mount(struct autofs_point *ap,
 			      what, fstype, fullpath);
 		}
 
-		if (ap->flags & (MOUNT_FLAG_SLAVE | MOUNT_FLAG_PRIVATE)) {
-			int flags = MS_SLAVE;
-
-			if (ap->flags & MOUNT_FLAG_PRIVATE)
-				flags = MS_PRIVATE;
-
-			/* The bind mount has succeeded but if the target
-			 * mount is propagation shared propagation of child
-			 * mounts (autofs offset mounts for example) back to
-			 * the target of the bind mount must be avoided or
-			 * autofs trigger mounts will deadlock.
-			 */
-			err = mount(NULL, fullpath, NULL, flags, NULL);
-			if (err) {
-				warn(ap->logopt,
-				     "failed to set propagation for %s",
-				     fullpath, root);
-			}
+		/* The bind mount has succeeded, now set the mount propagation.
+		 *
+		 * The default is propagation shared, change it if the master
+		 * map entry has a different option specified.
+		 */
+		flags = MS_SLAVE;
+		if (ap->flags & MOUNT_FLAG_PRIVATE)
+			flags = MS_PRIVATE;
+		else if (ap->flags & MOUNT_FLAG_SHARED)
+			flags = MS_SHARED;
+
+		/* Note: If the parent mount is propagation shared propagation
+		 *  of child mounts (autofs offset mounts for example) back to
+		 *  the target of the bind mount can happen in some cases and
+		 *  must be avoided or autofs trigger mounts will deadlock.
+		 */
+		err = mount(NULL, fullpath, NULL, flags, NULL);
+		if (err) {
+			warn(ap->logopt,
+			     "failed to set propagation for %s",
+			     fullpath, root);
 		}
 
 		return 0;
--- autofs-5.1.4.orig/CHANGELOG
+++ autofs-5.1.4/CHANGELOG
@@ -114,6 +114,7 @@ xx/xx/2018 autofs-5.1.5
 - use defines for expire type.
 - remove unused function dump_master().
 - fix additional typing errors.
+- make bind mounts propagation slave by default.
 
 19/12/2017 autofs-5.1.4
 - fix spec file url.