autofs-5.1.6 - fix a regression with map instance lookup From: Ian Kent Commit b66deff4241d ("autofs-5.1.3 - fix possible map instance memory leak") introduced a regression. The change didn't fix the memory leak and also failed to fix the race updating the map instance list that caused it. To fix this get rid of the horible temporary map usage and update the instance list in place. Doing this causes some additional allocations and frees but is somewhat simpler overall and avoids the race. Fixes: b66deff4241d ("autofs-5.1.3 - fix possible map instance memory leak") Signed-off-by: Ian Kent --- CHANGELOG | 1 daemon/lookup.c | 188 ++++++++++++++++++++++--------------------------------- 2 files changed, 76 insertions(+), 113 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 214ee8bb..3eb34097 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,6 +3,7 @@ xx/xx/2020 autofs-5.1.7 - update ldap READMEs and schema definitions. - fix program map multi-mount lookup after mount fail. - fix browse dir not re-created on symlink expire. +- fix a regression with map instance lookup. 07/10/2019 autofs-5.1.6 - support strictexpire mount option. diff --git a/daemon/lookup.c b/daemon/lookup.c index 60a48f3f..8d54c465 100644 --- a/daemon/lookup.c +++ b/daemon/lookup.c @@ -64,6 +64,10 @@ static char *find_map_path(struct autofs_point *ap, struct map_source *map) char *search_path; struct stat st; + /* Absolute path, just return a copy */ + if (mname[0] == '/') + return strdup(mname); + /* * This is different to the way it is in amd. * autofs will always try to locate maps in AUTOFS_MAP_DIR @@ -373,14 +377,27 @@ static int read_file_source_instance(struct autofs_point *ap, struct map_source char src_prog[] = "program"; struct stat st; char *type, *format; + char *path; + + if (map->argc < 1) { + error(ap->logopt, "invalid arguments for autofs_point"); + return NSS_STATUS_UNKNOWN; + } - if (stat(map->argv[0], &st) == -1) { - warn(ap->logopt, "file map %s not found", map->argv[0]); + path = find_map_path(ap, map); + if (!path) + return NSS_STATUS_UNKNOWN; + + if (stat(path, &st) == -1) { + warn(ap->logopt, "file map %s not found", path); + free(path); return NSS_STATUS_NOTFOUND; } - if (!S_ISREG(st.st_mode)) + if (!S_ISREG(st.st_mode)) { + free(path); return NSS_STATUS_NOTFOUND; + } if (st.st_mode & __S_IEXEC) type = src_prog; @@ -391,9 +408,23 @@ static int read_file_source_instance(struct autofs_point *ap, struct map_source instance = master_find_source_instance(map, type, format, 0, NULL); if (!instance) { - int argc = map->argc; - const char **argv = map->argv; + const char **argv; + int argc; + + argc = map->argc; + argv = copy_argv(map->argc, map->argv); + if (!argv) { + error(ap->logopt, "failed to copy args"); + free(path); + return NSS_STATUS_UNKNOWN; + } + if (argv[0]) + free((char *) argv[0]); + argv[0] = path; + path = NULL; + instance = master_add_source_instance(map, type, format, age, argc, argv); + free_argv(argc, argv); if (!instance) return NSS_STATUS_UNAVAIL; instance->recurse = map->recurse; @@ -401,6 +432,9 @@ static int read_file_source_instance(struct autofs_point *ap, struct map_source } instance->stale = map->stale; + if (path) + free(path); + return do_read_map(ap, instance, age); } @@ -426,14 +460,6 @@ static int read_source_instance(struct autofs_point *ap, struct map_source *map, return do_read_map(ap, instance, age); } -static void argv_cleanup(void *arg) -{ - struct map_source *tmap = (struct map_source *) arg; - /* path is freed in free_argv */ - free_argv(tmap->argc, tmap->argv); - return; -} - static int lookup_map_read_map(struct autofs_point *ap, struct map_source *map, time_t age) { @@ -476,16 +502,11 @@ static int lookup_map_read_map(struct autofs_point *ap, static enum nsswitch_status read_map_source(struct nss_source *this, struct autofs_point *ap, struct map_source *map, time_t age) { - enum nsswitch_status result; - struct map_source tmap; - char *path; - if (strcasecmp(this->source, "files")) { return read_source_instance(ap, map, this->source, age); } /* - * autofs built-in map for nsswitch "files" is "file". * This is a special case as we need to append the * normal location to the map name. * note: It's invalid to specify a relative path. @@ -496,50 +517,7 @@ static enum nsswitch_status read_map_source(struct nss_source *this, return NSS_STATUS_NOTFOUND; } - this->source[4] = '\0'; - tmap.flags = map->flags; - tmap.type = this->source; - tmap.format = map->format; - tmap.name = map->name; - tmap.lookup = map->lookup; - tmap.mc = map->mc; - tmap.instance = map->instance; - tmap.exp_timeout = map->exp_timeout; - tmap.recurse = map->recurse; - tmap.depth = map->depth; - tmap.stale = map->stale; - tmap.argc = 0; - tmap.argv = NULL; - - path = find_map_path(ap, map); - if (!path) - return NSS_STATUS_UNKNOWN; - - if (map->argc >= 1) { - tmap.argc = map->argc; - tmap.argv = copy_argv(map->argc, map->argv); - if (!tmap.argv) { - error(ap->logopt, "failed to copy args"); - free(path); - return NSS_STATUS_UNKNOWN; - } - if (tmap.argv[0]) - free((char *) tmap.argv[0]); - tmap.argv[0] = path; - } else { - error(ap->logopt, "invalid arguments for autofs_point"); - free(path); - return NSS_STATUS_UNKNOWN; - } - - pthread_cleanup_push(argv_cleanup, &tmap); - result = read_file_source_instance(ap, &tmap, age); - pthread_cleanup_pop(1); - - if (!map->instance) - map->instance = tmap.instance; - - return result; + return read_file_source_instance(ap, map, age); } int lookup_nss_read_map(struct autofs_point *ap, struct map_source *source, time_t age) @@ -925,17 +903,30 @@ static int lookup_name_file_source_instance(struct autofs_point *ap, struct map_ time_t age = monotonic_time(NULL); struct stat st; char *type, *format; + char *path; if (*name == '/' && map->flags & MAP_FLAG_FORMAT_AMD) return lookup_amd_instance(ap, map, name, name_len); - if (stat(map->argv[0], &st) == -1) { + if (map->argc < 1) { + error(ap->logopt, "invalid arguments for autofs_point"); + return NSS_STATUS_UNKNOWN; + } + + path = find_map_path(ap, map); + if (!path) + return NSS_STATUS_UNKNOWN; + + if (stat(path, &st) == -1) { debug(ap->logopt, "file map not found"); + free(path); return NSS_STATUS_NOTFOUND; } - if (!S_ISREG(st.st_mode)) + if (!S_ISREG(st.st_mode)) { + free(path); return NSS_STATUS_NOTFOUND; + } if (st.st_mode & __S_IEXEC) type = src_prog; @@ -946,15 +937,32 @@ static int lookup_name_file_source_instance(struct autofs_point *ap, struct map_ instance = master_find_source_instance(map, type, format, 0, NULL); if (!instance) { - int argc = map->argc; - const char **argv = map->argv; + const char **argv; + int argc; + + argc = map->argc; + argv = copy_argv(map->argc, map->argv); + if (!argv) { + error(ap->logopt, "failed to copy args"); + free(path); + return NSS_STATUS_UNKNOWN; + } + if (argv[0]) + free((char *) argv[0]); + argv[0] = path; + path = NULL; + instance = master_add_source_instance(map, type, format, age, argc, argv); + free_argv(argc, argv); if (!instance) return NSS_STATUS_NOTFOUND; instance->recurse = map->recurse; instance->depth = map->depth; } + if (path) + free(path); + return do_lookup_mount(ap, instance, name, name_len); } @@ -1030,10 +1038,6 @@ static enum nsswitch_status lookup_map_name(struct nss_source *this, struct autofs_point *ap, struct map_source *map, const char *name, int name_len) { - enum nsswitch_status result; - struct map_source tmap; - char *path; - if (strcasecmp(this->source, "files")) return lookup_name_source_instance(ap, map, this->source, name, name_len); @@ -1050,49 +1054,7 @@ static enum nsswitch_status lookup_map_name(struct nss_source *this, return NSS_STATUS_NOTFOUND; } - this->source[4] = '\0'; - tmap.flags = map->flags; - tmap.type = this->source; - tmap.format = map->format; - tmap.name = map->name; - tmap.mc = map->mc; - tmap.instance = map->instance; - tmap.exp_timeout = map->exp_timeout; - tmap.recurse = map->recurse; - tmap.depth = map->depth; - tmap.argc = 0; - tmap.argv = NULL; - - path = find_map_path(ap, map); - if (!path) - return NSS_STATUS_UNKNOWN; - - if (map->argc >= 1) { - tmap.argc = map->argc; - tmap.argv = copy_argv(map->argc, map->argv); - if (!tmap.argv) { - error(ap->logopt, "failed to copy args"); - free(path); - return NSS_STATUS_UNKNOWN; - } - if (tmap.argv[0]) - free((char *) tmap.argv[0]); - tmap.argv[0] = path; - } else { - error(ap->logopt, "invalid arguments for autofs_point"); - free(path); - return NSS_STATUS_UNKNOWN; - } - - result = lookup_name_file_source_instance(ap, &tmap, name, name_len); - - if (!map->instance) - map->instance = tmap.instance; - - /* path is freed in free_argv */ - free_argv(tmap.argc, tmap.argv); - - return result; + return lookup_name_file_source_instance(ap, map, name, name_len); } static struct map_source *lookup_get_map_source(struct master_mapent *entry)