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

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