Blame SOURCES/autofs-5.1.7-fix-inconsistent-locking-in-parse_mount.patch

49b67f
autofs-5.1.7 - fix inconsistent locking in parse_mount()
49b67f
49b67f
From: Ian Kent <raven@themaw.net>
49b67f
49b67f
Some map entry cache locking inconsistencies have crept in.
49b67f
49b67f
In parse_mount() of the sun format parser the cache read lock is too
49b67f
heavily used and has too broad a scope. This has lead to some operations
49b67f
that should hold the write lock being called with only the read lock.
49b67f
49b67f
Signed-off-by: Ian Kent <raven@themaw.net>
49b67f
---
49b67f
 CHANGELOG           |    1 
49b67f
 lib/mounts.c        |    9 +++++++-
49b67f
 modules/parse_sun.c |   53 ++++++++++++++++++++++++++++++++--------------------
49b67f
 3 files changed, 42 insertions(+), 21 deletions(-)
49b67f
49b67f
--- autofs-5.1.4.orig/CHANGELOG
49b67f
+++ autofs-5.1.4/CHANGELOG
49b67f
@@ -17,6 +17,7 @@
49b67f
 - fix inconsistent locking in umount_subtree_mounts().
49b67f
 - fix return from umount_subtree_mounts() on offset list delete.
49b67f
 - pass mapent_cache to update_offset_entry().
49b67f
+- fix inconsistent locking in parse_mount().
49b67f
 
49b67f
 xx/xx/2018 autofs-5.1.5
49b67f
 - fix flag file permission.
49b67f
--- autofs-5.1.4.orig/lib/mounts.c
49b67f
+++ autofs-5.1.4/lib/mounts.c
49b67f
@@ -2485,6 +2485,12 @@ static int do_mount_autofs_offset(struct
49b67f
 		else {
49b67f
 			debug(ap->logopt, "ignoring \"nohide\" trigger %s",
49b67f
 			      oe->key);
49b67f
+			/*
49b67f
+			 * Ok, so we shouldn't modify the mapent but
49b67f
+			 * mount requests are blocked at a point above
49b67f
+			 * this and expire only uses the mapent key or
49b67f
+			 * holds the cache write lock.
49b67f
+			 */
49b67f
 			free(oe->mapent);
49b67f
 			oe->mapent = NULL;
49b67f
 		}
49b67f
@@ -2628,7 +2634,8 @@ static int do_umount_offset(struct autof
49b67f
 			/*
49b67f
 			 * Ok, so we shouldn't modify the mapent but
49b67f
 			 * mount requests are blocked at a point above
49b67f
-			 * this and expire only uses the mapent key.
49b67f
+			 * this and expire only uses the mapent key or
49b67f
+			 * holds the cache write lock.
49b67f
 			 */
49b67f
 			if (oe->mapent) {
49b67f
 				free(oe->mapent);
49b67f
--- autofs-5.1.4.orig/modules/parse_sun.c
49b67f
+++ autofs-5.1.4/modules/parse_sun.c
49b67f
@@ -853,10 +853,12 @@ update_offset_entry(struct autofs_point
49b67f
 			strcpy(m_mapent, loc);
49b67f
 	}
49b67f
 
49b67f
+	cache_writelock(mc);
49b67f
 	ret = cache_update_offset(mc, name, m_key, m_mapent, age);
49b67f
 
49b67f
 	if (!cache_set_offset_parent(mc, m_key))
49b67f
 		error(ap->logopt, "failed to set offset parent");
49b67f
+	cache_unlock(mc);
49b67f
 
49b67f
 	if (ret == CHE_DUPLICATE) {
49b67f
 		warn(ap->logopt, MODPREFIX
49b67f
@@ -1130,14 +1132,22 @@ static void cleanup_multi_triggers(struc
49b67f
 	return;
49b67f
 }
49b67f
 
49b67f
-static int mount_subtree(struct autofs_point *ap, struct mapent *me,
49b67f
+static int mount_subtree(struct autofs_point *ap, struct mapent_cache *mc,
49b67f
 			 const char *name, char *loc, char *options, void *ctxt)
49b67f
 {
49b67f
+	struct mapent *me;
49b67f
 	struct mapent *ro;
49b67f
 	char *mm_root, *mm_base, *mm_key;
49b67f
 	unsigned int mm_root_len;
49b67f
 	int start, ret = 0, rv;
49b67f
 
49b67f
+	cache_readlock(mc);
49b67f
+	me = cache_lookup_distinct(mc, name);
49b67f
+	if (!me) {
49b67f
+		cache_unlock(mc);
49b67f
+		return 0;
49b67f
+	}
49b67f
+
49b67f
 	rv = 0;
49b67f
 
49b67f
 	mm_key = me->multi->key;
49b67f
@@ -1182,9 +1192,12 @@ static int mount_subtree(struct autofs_p
49b67f
 			rv = parse_mapent(ro->mapent,
49b67f
 				options, &myoptions, &ro_loc, ap->logopt);
49b67f
 			if (!rv) {
49b67f
+				cache_unlock(mc);
49b67f
 				warn(ap->logopt,
49b67f
 				      MODPREFIX "failed to parse root offset");
49b67f
-				cache_delete_offset_list(me->mc, name);
49b67f
+				cache_writelock(mc);
49b67f
+				cache_delete_offset_list(mc, name);
49b67f
+				cache_unlock(mc);
49b67f
 				return 1;
49b67f
 			}
49b67f
 			ro_len = 0;
49b67f
@@ -1201,9 +1214,10 @@ static int mount_subtree(struct autofs_p
49b67f
 		if ((ro && rv == 0) || rv <= 0) {
49b67f
 			ret = mount_multi_triggers(ap, me, mm_root, start, mm_base);
49b67f
 			if (ret == -1) {
49b67f
+				cleanup_multi_triggers(ap, me, mm_root, start, mm_base);
49b67f
+				cache_unlock(mc);
49b67f
 				error(ap->logopt, MODPREFIX
49b67f
 					 "failed to mount offset triggers");
49b67f
-				cleanup_multi_triggers(ap, me, mm_root, start, mm_base);
49b67f
 				return 1;
49b67f
 			}
49b67f
 		}
49b67f
@@ -1219,9 +1233,10 @@ static int mount_subtree(struct autofs_p
49b67f
 		if (rv == 0) {
49b67f
 			ret = mount_multi_triggers(ap, me->multi, name, start, mm_base);
49b67f
 			if (ret == -1) {
49b67f
+				cleanup_multi_triggers(ap, me, name, start, mm_base);
49b67f
+				cache_unlock(mc);
49b67f
 				error(ap->logopt, MODPREFIX
49b67f
 					 "failed to mount offset triggers");
49b67f
-				cleanup_multi_triggers(ap, me, name, start, mm_base);
49b67f
 				return 1;
49b67f
 			}
49b67f
 		} else if (rv < 0) {
49b67f
@@ -1229,8 +1244,11 @@ static int mount_subtree(struct autofs_p
49b67f
 			unsigned int mm_root_base_len = mm_root_len + strlen(mm_base) + 1;
49b67f
 	
49b67f
 			if (mm_root_base_len > PATH_MAX) {
49b67f
+				cache_unlock(mc);
49b67f
 				warn(ap->logopt, MODPREFIX "path too long");
49b67f
-				cache_delete_offset_list(me->mc, name);
49b67f
+				cache_writelock(mc);
49b67f
+				cache_delete_offset_list(mc, name);
49b67f
+				cache_unlock(mc);
49b67f
 				return 1;
49b67f
 			}
49b67f
 
49b67f
@@ -1239,13 +1257,15 @@ static int mount_subtree(struct autofs_p
49b67f
 
49b67f
 			ret = mount_multi_triggers(ap, me->multi, mm_root_base, start, mm_base);
49b67f
 			if (ret == -1) {
49b67f
+				cleanup_multi_triggers(ap, me, mm_root, start, mm_base);
49b67f
+				cache_unlock(mc);
49b67f
 				error(ap->logopt, MODPREFIX
49b67f
 					 "failed to mount offset triggers");
49b67f
-				cleanup_multi_triggers(ap, me, mm_root, start, mm_base);
49b67f
 				return 1;
49b67f
 			}
49b67f
 		}
49b67f
 	}
49b67f
+	cache_unlock(mc);
49b67f
 
49b67f
 	/* Mount for base of tree failed */
49b67f
 	if (rv > 0)
49b67f
@@ -1486,7 +1506,6 @@ dont_expand:
49b67f
 			return 1;
49b67f
 		}
49b67f
 
49b67f
-		cache_multi_writelock(me);
49b67f
 		/* So we know we're the multi-mount root */
49b67f
 		if (!me->multi)
49b67f
 			me->multi = me;
49b67f
@@ -1511,14 +1530,13 @@ dont_expand:
49b67f
 			if (source->flags & MAP_FLAG_FORMAT_AMD) {
49b67f
 				free(options);
49b67f
 				free(pmapent);
49b67f
-				cache_multi_unlock(me);
49b67f
 				cache_unlock(mc);
49b67f
 				pthread_setcancelstate(cur_state, NULL);
49b67f
 				return 0;
49b67f
 			}
49b67f
 		}
49b67f
-
49b67f
 		age = me->age;
49b67f
+		cache_unlock(mc);
49b67f
 
49b67f
 		/* It's a multi-mount; deal with it */
49b67f
 		do {
49b67f
@@ -1539,8 +1557,8 @@ dont_expand:
49b67f
 
49b67f
 			if (!path) {
49b67f
 				warn(ap->logopt, MODPREFIX "null path or out of memory");
49b67f
+				cache_writelock(mc);
49b67f
 				cache_delete_offset_list(mc, name);
49b67f
-				cache_multi_unlock(me);
49b67f
 				cache_unlock(mc);
49b67f
 				free(options);
49b67f
 				free(pmapent);
49b67f
@@ -1556,8 +1574,8 @@ dont_expand:
49b67f
 
49b67f
 			l = parse_mapent(p, options, &myoptions, &loc, ap->logopt);
49b67f
 			if (!l) {
49b67f
+				cache_writelock(mc);
49b67f
 				cache_delete_offset_list(mc, name);
49b67f
-				cache_multi_unlock(me);
49b67f
 				cache_unlock(mc);
49b67f
 				free(path);
49b67f
 				free(options);
49b67f
@@ -1575,8 +1593,8 @@ dont_expand:
49b67f
 
49b67f
 			if (status != CHE_OK) {
49b67f
 				warn(ap->logopt, MODPREFIX "error adding multi-mount");
49b67f
+				cache_writelock(mc);
49b67f
 				cache_delete_offset_list(mc, name);
49b67f
-				cache_multi_unlock(me);
49b67f
 				cache_unlock(mc);
49b67f
 				free(path);
49b67f
 				free(options);
49b67f
@@ -1594,10 +1612,7 @@ dont_expand:
49b67f
 			free(myoptions);
49b67f
 		} while (*p == '/' || (*p == '"' && *(p + 1) == '/'));
49b67f
 
49b67f
-		rv = mount_subtree(ap, me, name, NULL, options, ctxt);
49b67f
-
49b67f
-		cache_multi_unlock(me);
49b67f
-		cache_unlock(mc);
49b67f
+		rv = mount_subtree(ap, mc, name, NULL, options, ctxt);
49b67f
 
49b67f
 		free(options);
49b67f
 		free(pmapent);
49b67f
@@ -1618,6 +1633,7 @@ dont_expand:
49b67f
 		cache_readlock(mc);
49b67f
 		if (*name == '/' &&
49b67f
 		   (me = cache_lookup_distinct(mc, name)) && me->multi) {
49b67f
+			cache_unlock(mc);
49b67f
 			loc = strdup(p);
49b67f
 			if (!loc) {
49b67f
 				free(options);
49b67f
@@ -1626,10 +1642,7 @@ dont_expand:
49b67f
 				warn(ap->logopt, MODPREFIX "out of memory");
49b67f
 				return 1;
49b67f
 			}
49b67f
-			cache_multi_writelock(me);
49b67f
-			rv = mount_subtree(ap, me, name, loc, options, ctxt);
49b67f
-			cache_multi_unlock(me);
49b67f
-			cache_unlock(mc);
49b67f
+			rv = mount_subtree(ap, mc, name, loc, options, ctxt);
49b67f
 			free(loc);
49b67f
 			free(options);
49b67f
 			free(pmapent);