autofs-5.1.8 - fix deadlock in lookups From: Ian Kent After adding locking to fix a crash during lookups we're seeing a deadlock becuase of recursive calls. But once the lookup is open we shouldn't need to open it again during the recursive call, fix it based on that. Signed-off-by: Ian Kent --- CHANGELOG | 1 + daemon/lookup.c | 62 +++++++++++++++++++++++++++++++++------------------ daemon/master.c | 8 +++++++ include/master.h | 1 + modules/parse_amd.c | 2 +- 5 files changed, 51 insertions(+), 23 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5f7e39bd..c24a7c82 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -58,6 +58,7 @@ - define LDAP_DEPRECATED during LDAP configure check. - fix return status of mount_autofs(). - don't close lookup at umount. +- fix deadlock in lookups. 19/10/2021 autofs-5.1.8 - add xdr_exports(). diff --git a/daemon/lookup.c b/daemon/lookup.c index 29f43d24..33fca23a 100644 --- a/daemon/lookup.c +++ b/daemon/lookup.c @@ -320,31 +320,49 @@ static int do_read_map(struct autofs_point *ap, struct map_source *map, time_t a struct lookup_mod *lookup; int status; - pthread_cleanup_push(map_module_lock_cleanup, map); - map_module_writelock(map); - if (!map->lookup) { - status = open_lookup(map->type, "", map->format, - map->argc, map->argv, &lookup); - if (status == NSS_STATUS_SUCCESS) - map->lookup = lookup; - else - debug(ap->logopt, - "lookup module %s open failed", map->type); - } else { - status = map->lookup->lookup_reinit(map->format, - map->argc, map->argv, - &map->lookup->context); - if (status) - warn(ap->logopt, - "lookup module %s reinit failed", map->type); - } - pthread_cleanup_pop(1); - if (status != NSS_STATUS_SUCCESS) - return status; - if (!map->stale) return NSS_STATUS_SUCCESS; + /* If this readmap is the result of trying to mount a submount + * the readlock may already be held if the map is the same as + * that of the caller. In that case the map has already been + * read so just skip the map open/reinit. + */ + status = map_module_try_writelock(map); + if (status) { + if (!map->lookup) { + error(ap->logopt, "map module lock not held as expected"); + return NSS_STATUS_UNAVAIL; + } + } else { + if (!map->lookup) { + pthread_cleanup_push(map_module_lock_cleanup, map); + status = open_lookup(map->type, "", map->format, + map->argc, map->argv, &lookup); + pthread_cleanup_pop(0); + if (status != NSS_STATUS_SUCCESS) { + map_module_unlock(map); + debug(ap->logopt, + "lookup module %s open failed", map->type); + return status; + } + map->lookup = lookup; + } else { + pthread_cleanup_push(map_module_lock_cleanup, map); + status = map->lookup->lookup_reinit(map->format, + map->argc, map->argv, + &map->lookup->context); + pthread_cleanup_pop(0); + if (status) { + map_module_unlock(map); + warn(ap->logopt, + "lookup module %s reinit failed", map->type); + return status; + } + } + map_module_unlock(map); + } + master_source_current_wait(ap->entry); ap->entry->current = map; diff --git a/daemon/master.c b/daemon/master.c index 3f540d01..c082d319 100644 --- a/daemon/master.c +++ b/daemon/master.c @@ -73,6 +73,14 @@ void map_module_writelock(struct map_source *map) fatal(status); } +int map_module_try_writelock(struct map_source *map) +{ + int status = pthread_rwlock_trywrlock(&map->module_lock); + if (status && status != EBUSY && status != EDEADLK) + fatal(status); + return status; +} + void map_module_readlock(struct map_source *map) { int status = pthread_rwlock_rdlock(&map->module_lock); diff --git a/include/master.h b/include/master.h index cea2e6e7..597b3213 100644 --- a/include/master.h +++ b/include/master.h @@ -128,6 +128,7 @@ int master_list_empty(struct master *); int master_done(struct master *); int master_kill(struct master *); void map_module_writelock(struct map_source *map); +int map_module_try_writelock(struct map_source *map); void map_module_readlock(struct map_source *map); void map_module_unlock(struct map_source *map); void map_module_lock_cleanup(void *arg); diff --git a/modules/parse_amd.c b/modules/parse_amd.c index e2dd0b33..e76edf31 100644 --- a/modules/parse_amd.c +++ b/modules/parse_amd.c @@ -1398,7 +1398,7 @@ static int do_host_mount(struct autofs_point *ap, const char *name, cache_unlock(source->mc); master_source_current_wait(ap->entry); - ap->entry->current = source; + ap->entry->current = instance; map_module_readlock(instance); lookup = instance->lookup;