Blame SOURCES/autofs-5.1.8-fix-deadlock-with-hosts-map-reload.patch

591f3a
autofs-5.1.8 - fix deadlock with hosts map reload
591f3a
591f3a
From: Ian Kent <raven@themaw.net>
591f3a
591f3a
When reloading maps the hosts map calls lookup method ->parse_mount()
591f3a
for each multi-mount root entry in the map (each host) while holding
591f3a
the cache read lock which leads to a cache lock deadlock.
591f3a
591f3a
Remove the need to hold the cache read lock by creating an independent
591f3a
list of entries for the update so the lock doesn't need to be taken.
591f3a
591f3a
Signed-off-by: Ian Kent <raven@themaw.net>
591f3a
---
591f3a
 CHANGELOG              |    1 
591f3a
 modules/lookup_hosts.c |  100 ++++++++++++++++++++++++++++++++++++++++---------
591f3a
 2 files changed, 83 insertions(+), 18 deletions(-)
591f3a
591f3a
--- autofs-5.1.4.orig/CHANGELOG
591f3a
+++ autofs-5.1.4/CHANGELOG
591f3a
@@ -100,6 +100,7 @@
591f3a
 - serialise lookup module open and reinit.
591f3a
 - coverity fix for invalid access.
591f3a
 - fix hosts map deadlock on restart.
591f3a
+- fix deadlock with hosts map reload.
591f3a
 
591f3a
 xx/xx/2018 autofs-5.1.5
591f3a
 - fix flag file permission.
591f3a
--- autofs-5.1.4.orig/modules/lookup_hosts.c
591f3a
+++ autofs-5.1.4/modules/lookup_hosts.c
591f3a
@@ -201,10 +201,72 @@ static int do_parse_mount(struct autofs_
591f3a
 	return NSS_STATUS_SUCCESS;
591f3a
 }
591f3a
 
591f3a
+struct update_context {
591f3a
+	char *key;
591f3a
+	int key_len;
591f3a
+	char *entry;
591f3a
+	struct update_context *next;
591f3a
+};
591f3a
+
591f3a
+static int add_update_entry(struct update_context **entries, struct mapent *me)
591f3a
+{
591f3a
+	struct update_context *upd;
591f3a
+	char *key, *ent;
591f3a
+
591f3a
+	key = strdup(me->key);
591f3a
+	if (!key)
591f3a
+		return 0;
591f3a
+
591f3a
+	ent = strdup(me->mapent);
591f3a
+	if (!ent) {
591f3a
+		free(key);
591f3a
+		return 0;
591f3a
+	}
591f3a
+
591f3a
+	upd = malloc(sizeof(struct update_context));
591f3a
+	if (!upd) {
591f3a
+		free(ent);
591f3a
+		free(key);
591f3a
+		return 0;
591f3a
+	}
591f3a
+
591f3a
+	upd->key = key;
591f3a
+	upd->key_len = me->len;
591f3a
+	upd->entry = ent;
591f3a
+	upd->next = NULL;
591f3a
+	if (*entries)
591f3a
+		(*entries)->next = upd;
591f3a
+	*entries = upd;
591f3a
+
591f3a
+	return 1;
591f3a
+}
591f3a
+
591f3a
+static void free_update_entries(struct update_context *entries)
591f3a
+{
591f3a
+	struct update_context *this = entries;
591f3a
+
591f3a
+	while (this) {
591f3a
+		struct update_context *next = this->next;
591f3a
+		free(this->key);
591f3a
+		free(this->entry);
591f3a
+		free(this);
591f3a
+		this = next;
591f3a
+	}
591f3a
+}
591f3a
+
591f3a
+void entries_cleanup(void *arg)
591f3a
+{
591f3a
+	struct update_context *entries = arg;
591f3a
+
591f3a
+	free_update_entries(entries);
591f3a
+}
591f3a
+
591f3a
 static void update_hosts_mounts(struct autofs_point *ap,
591f3a
 				struct map_source *source, time_t age,
591f3a
 				struct lookup_context *ctxt)
591f3a
 {
591f3a
+	struct update_context *head = NULL;
591f3a
+	struct update_context *entries = NULL;
591f3a
 	struct mapent_cache *mc;
591f3a
 	struct mapent *me;
591f3a
 	char *mapent;
591f3a
@@ -212,6 +274,8 @@ static void update_hosts_mounts(struct a
591f3a
 
591f3a
 	mc = source->mc;
591f3a
 
591f3a
+	pthread_cleanup_push(entries_cleanup, head);
591f3a
+
591f3a
 	pthread_cleanup_push(cache_lock_cleanup, mc);
591f3a
 	cache_writelock(mc);
591f3a
 	me = cache_lookup_first(mc);
591f3a
@@ -224,39 +288,39 @@ static void update_hosts_mounts(struct a
591f3a
 
591f3a
 		mapent = get_exports(ap, me->key);
591f3a
 		if (mapent) {
591f3a
-			cache_update(mc, source, me->key, mapent, age);
591f3a
+			int ret;
591f3a
+
591f3a
+			ret = cache_update(mc, source, me->key, mapent, age);
591f3a
 			free(mapent);
591f3a
+			if (!IS_MM_ROOT(me))
591f3a
+				goto next;
591f3a
+			if (ret != CHE_FAIL) {
591f3a
+				if (!add_update_entry(&entries, me))
591f3a
+					warn(ap->logopt, MODPREFIX
591f3a
+					     "failed to add update entry for %s", me->key);
591f3a
+				else if (!head)
591f3a
+					head = entries;
591f3a
+			}
591f3a
 		}
591f3a
 next:
591f3a
 		me = cache_lookup_next(mc, me);
591f3a
 	}
591f3a
 	pthread_cleanup_pop(1);
591f3a
 
591f3a
-	pthread_cleanup_push(cache_lock_cleanup, mc);
591f3a
-	cache_readlock(mc);
591f3a
-	me = cache_lookup_first(mc);
591f3a
-	while (me) {
591f3a
-		/*
591f3a
-		 * Hosts map entry not yet expanded, already expired
591f3a
-		 * or not the base of the tree
591f3a
-		 */
591f3a
-		if (!IS_MM(me) || !IS_MM_ROOT(me))
591f3a
-			goto cont;
591f3a
-
591f3a
+	while (head) {
591f3a
 		debug(ap->logopt, MODPREFIX
591f3a
-		      "attempt to update exports for %s", me->key);
591f3a
+		      "attempt to update exports for %s", head->key);
591f3a
 
591f3a
 		master_source_current_wait(ap->entry);
591f3a
 		ap->entry->current = source;
591f3a
 		ap->flags |= MOUNT_FLAG_REMOUNT;
591f3a
-		ret = ctxt->parse->parse_mount(ap, me->key, strlen(me->key),
591f3a
-					       me->mapent, ctxt->parse->context);
591f3a
+		ret = ctxt->parse->parse_mount(ap, head->key, strlen(head->key),
591f3a
+					       head->entry, ctxt->parse->context);
591f3a
 		if (ret)
591f3a
 			warn(ap->logopt, MODPREFIX
591f3a
-			     "failed to parse mount %s", me->mapent);
591f3a
+			     "failed to parse mount %s", head->entry);
591f3a
 		ap->flags &= ~MOUNT_FLAG_REMOUNT;
591f3a
-cont:
591f3a
-		me = cache_lookup_next(mc, me);
591f3a
+		head = head->next;
591f3a
 	}
591f3a
 	pthread_cleanup_pop(1);
591f3a
 }