autofs-5.1.7 - fix offset entries order

From: Ian Kent <raven@themaw.net>

While it's rare it's possible that a mapent entry might not have
it's offsets in shortest to longest path order.

If this happens adding an entry to the mapent tree can result in
an incorrect tree topology that doesn't work. That's because adding
tree entries ensures that nodes in a sub-tree are placed below the
containing node so the containing node must be present for that to
work. This topology is critical to the performance of map entries
that have a very large number of offsets such as an NFS server with
many exports.

There's no other choice but make a traversal after the offset entries
have all been added to create the mapent tree.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 CHANGELOG           |    1 
 include/automount.h |    1 
 lib/cache.c         |    1 
 modules/parse_sun.c |   74 +++++++++++++++++++++++++++++++++++++++++-----------
 4 files changed, 62 insertions(+), 15 deletions(-)

--- autofs-5.1.4.orig/CHANGELOG
+++ autofs-5.1.4/CHANGELOG
@@ -68,6 +68,7 @@
 - add ext_mount_hash_mutex lock helpers.
 - fix amd section mounts map reload.
 - fix amd hosts mount expire.
+- fix offset entries order.
 
 xx/xx/2018 autofs-5.1.5
 - fix flag file permission.
--- autofs-5.1.4.orig/include/automount.h
+++ autofs-5.1.4/include/automount.h
@@ -169,6 +169,7 @@ struct mapent {
 	/* Parent nesting point within multi-mount */
 	struct tree_node *mm_parent;
 	struct tree_node node;
+	struct list_head work;
 	char *key;
 	size_t len;
 	char *mapent;
--- autofs-5.1.4.orig/lib/cache.c
+++ autofs-5.1.4/lib/cache.c
@@ -559,6 +559,7 @@ int cache_add(struct mapent_cache *mc, s
 	me->mm_parent = NULL;
 	INIT_TREE_NODE(&me->node);
 	INIT_LIST_HEAD(&me->ino_index);
+	INIT_LIST_HEAD(&me->work);
 	me->ioctlfd = -1;
 	me->dev = (dev_t) -1;
 	me->ino = (ino_t) -1;
--- autofs-5.1.4.orig/modules/parse_sun.c
+++ autofs-5.1.4/modules/parse_sun.c
@@ -791,14 +791,15 @@ static int check_is_multi(const char *ma
 
 static int
 update_offset_entry(struct autofs_point *ap,
-		    struct mapent_cache *mc, const char *name,
-		    const char *m_root, int m_root_len,
+		    struct mapent_cache *mc, struct list_head *offsets,
+		    const char *name, const char *m_root, int m_root_len,
 		    const char *m_offset, const char *myoptions,
 		    const char *loc, time_t age)
 {
 	char m_key[PATH_MAX + 1];
 	char m_mapent[MAPENT_MAX_LEN + 1];
 	int o_len, m_key_len, m_options_len, m_mapent_len;
+	struct mapent *me;
 	int ret;
 
 	memset(m_mapent, 0, MAPENT_MAX_LEN + 1);
@@ -864,8 +865,29 @@ update_offset_entry(struct autofs_point
 	cache_writelock(mc);
 	ret = cache_update_offset(mc, name, m_key, m_mapent, age);
 
-	if (!tree_mapent_add_node(mc, name, m_key))
-		error(ap->logopt, "failed to add offset %s to tree", m_key);
+	me = cache_lookup_distinct(mc, m_key);
+	if (me && list_empty(&me->work)) {
+		struct list_head *last;
+
+		/* Offset entries really need to be in shortest to
+		 * longest path order. If not and the list of offsets
+		 * is large there will be a performace hit.
+		 */
+		list_for_each_prev(last, offsets) {
+			struct mapent *this;
+
+			this = list_entry(last, struct mapent, work);
+			if (me->len >= this->len) {
+				if (last->next == offsets)
+					list_add_tail(&me->work, offsets);
+				else
+					list_add_tail(&me->work, last);
+				break;
+			}
+		}
+		if (list_empty(&me->work))
+			list_add(&me->work, offsets);
+	}
 	cache_unlock(mc);
 
 	if (ret == CHE_DUPLICATE) {
@@ -1211,6 +1233,25 @@ static char *do_expandsunent(const char
 	return mapent;
 }
 
+static void cleanup_offset_entries(struct autofs_point *ap,
+				   struct mapent_cache *mc,
+				   struct list_head *offsets)
+{
+	struct mapent *me, *tmp;
+	int ret;
+
+	if (list_empty(offsets))
+		return;
+	cache_writelock(mc);
+	list_for_each_entry_safe(me, tmp, offsets, work) {
+		list_del(&me->work);
+		ret = cache_delete(mc, me->key);
+		if (ret != CHE_OK)
+			crit(ap->logopt, "failed to delete offset %s", me->key);
+	}
+	cache_unlock(mc);
+}
+
 /*
  * syntax is:
  *	[-options] location [location] ...
@@ -1230,7 +1271,8 @@ int parse_mount(struct autofs_point *ap,
 	char buf[MAX_ERR_BUF];
 	struct map_source *source;
 	struct mapent_cache *mc;
-	struct mapent *me;
+	struct mapent *me, *oe, *tmp;
+	LIST_HEAD(offsets);
 	char *pmapent, *options;
 	const char *p;
 	int mapent_len, rv = 0;
@@ -1446,9 +1488,7 @@ dont_expand:
 
 			if (!m_offset) {
 				warn(ap->logopt, MODPREFIX "null path or out of memory");
-				cache_writelock(mc);
-				tree_mapent_delete_offsets(mc, name);
-				cache_unlock(mc);
+				cleanup_offset_entries(ap, mc, &offsets);
 				free(options);
 				free(pmapent);
 				pthread_setcancelstate(cur_state, NULL);
@@ -1463,9 +1503,7 @@ dont_expand:
 
 			l = parse_mapent(p, options, &myoptions, &loc, ap->logopt);
 			if (!l) {
-				cache_writelock(mc);
-				tree_mapent_delete_offsets(mc, name);
-				cache_unlock(mc);
+				cleanup_offset_entries(ap, mc, &offsets);
 				free(m_offset);
 				free(options);
 				free(pmapent);
@@ -1476,15 +1514,13 @@ dont_expand:
 			p += l;
 			p = skipspace(p);
 
-			status = update_offset_entry(ap, mc,
+			status = update_offset_entry(ap, mc, &offsets,
 						     name, m_root, m_root_len,
 						     m_offset, myoptions, loc, age);
 
 			if (status != CHE_OK) {
 				warn(ap->logopt, MODPREFIX "error adding multi-mount");
-				cache_writelock(mc);
-				tree_mapent_delete_offsets(mc, name);
-				cache_unlock(mc);
+				cleanup_offset_entries(ap, mc, &offsets);
 				free(m_offset);
 				free(options);
 				free(pmapent);
@@ -1501,6 +1537,14 @@ dont_expand:
 			free(myoptions);
 		} while (*p == '/' || (*p == '"' && *(p + 1) == '/'));
 
+		cache_writelock(mc);
+		list_for_each_entry_safe(oe, tmp, &offsets, work) {
+			if (!tree_mapent_add_node(mc, name, oe->key))
+				error(ap->logopt, "failed to add offset %s to tree", oe->key);
+			list_del_init(&oe->work);
+		}
+		cache_unlock(mc);
+
 		rv = mount_subtree(ap, mc, name, NULL, options, ctxt);
 
 		free(options);