autofs-5.1.6 - fix a regression with map instance lookup

From: Ian Kent <raven@themaw.net>

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 <raven@themaw.net>

---

 CHANGELOG       |    1 

 daemon/lookup.c |  180 +++++++++++++++++++++++---------------------------------

 2 files changed, 76 insertions(+), 105 deletions(-)

--- autofs-5.1.4.orig/CHANGELOG

+++ autofs-5.1.4/CHANGELOG

@@ -76,6 +76,7 @@ xx/xx/2018 autofs-5.1.5

 - use local getmntent_r() in get_mnt_list().

 - use local getmntent_r() in tree_make_mnt_list().

 - fix missing initialization of autofs_point flags.

+- fix a regression with map instance lookup.

 19/12/2017 autofs-5.1.4

 - fix spec file url.

--- autofs-5.1.4.orig/daemon/lookup.c

+++ autofs-5.1.4/daemon/lookup.c

@@ -64,6 +64,10 @@ static char *find_map_path(struct autofs

 	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(str

 	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(str

 	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(str

 	}

 	instance->stale = map->stale;

+	if (path)

+		free(path);

+

 	return do_read_map(ap, instance, age);

 }

@@ -476,16 +510,11 @@ static int lookup_map_read_map(struct au

 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 +525,7 @@ static enum nsswitch_status read_map_sou

 		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 +911,30 @@ static int lookup_name_file_source_insta

 	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 +945,32 @@ static int lookup_name_file_source_insta

 	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 +1046,6 @@ static enum nsswitch_status lookup_map_n

 			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 +1062,7 @@ static enum nsswitch_status lookup_map_n

 		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)