From: Christos Zoulas <christos@zoulas.com>
Date: Thu, 10 Dec 2009 18:27:07 +0000 (-0500)
Subject: Make sure to remove nodes in the proper order when going down.
X-Git-Tag: am-utils-6_2_rc1~5
X-Git-Url: https://git.fsl.cs.stonybrook.edu/?a=commitdiff_plain;h=a29c14b0ec47b40669083eecb6d212a462e00e63;p=am-utils-6.2.git

Make sure to remove nodes in the proper order when going down.

Depending on what order the nodes got created it's possible that
the parent of a node has a bigger am_mapno (index in exported_ap[])
so that it gets freed before its child while the child's am_parent
pointer is still pointing to the already freed block of memory.

This change makes sure that umount_exported() cleans up all children
of a node first before freeing the node.

XXX: This patch did not apply cleanly because it pre-dated the am_al
patches.

From: Krisztian Kovacs <Kris.Kovacs@morganstanley.com>
---

diff --git a/ChangeLog b/ChangeLog
index 6ec3e48c..0379aadf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2009-12-10  Christos Zoulas <christos@zoulas.com>
 
+	* Make sure to remove nodes in the proper order when going
+	  down. Depending on what order the nodes got created it's
+	  possible that the parent of a node has a bigger am_mapno
+	  (index in exported_ap[]) so that it gets freed before
+	  its child while the child's am_parent pointer is still
+	  pointing to the already freed block of memory.
+	  This change makes sure that umount_exported() cleans up
+	  all children of a node first before freeing the node.
+	  From: Krisztian Kovacs <Kris.Kovacs@morganstanley.com>
+
 	* Fix Linux-specific stuff in ops_tmpfs.c
 	  AUTOFS_TMPFS_FS_FLAGS is defined only in the Linux-specific
 	  conf_linux.c file, so it cannot be built on Solaris.
diff --git a/amd/map.c b/amd/map.c
index ac707cc7..0b06f0d9 100644
--- a/amd/map.c
+++ b/amd/map.c
@@ -693,68 +693,80 @@ make_root_node(void)
 void
 umount_exported(void)
 {
-  int i;
+  int i, work_done;
 
-  for (i = last_used_map; i >= 0; --i) {
-    am_node *mp = exported_ap[i];
-    mntfs *mf;
+  do {
+    work_done = 0;
 
-    if (!mp)
-      continue;
+    for (i = last_used_map; i >= 0; --i) {
+      am_node *mp = exported_ap[i];
+      mntfs *mf;
 
-    mf = mp->am_al->al_mnt;
-    if (mf->mf_flags & MFF_UNMOUNTING) {
-      /*
-       * If this node is being unmounted then just ignore it.  However,
-       * this could prevent amd from finishing if the unmount gets blocked
-       * since the am_node will never be free'd.  am_unmounted needs
-       * telling about this possibility. - XXX
-       */
-      continue;
-    }
+      if (!mp)
+	continue;
 
-    if (!(mf->mf_fsflags & FS_DIRECTORY))
       /*
-       * When shutting down this had better
-       * look like a directory, otherwise it
-       * can't be unmounted!
+       * Wait for children to be removed first
        */
-      mk_fattr(&mp->am_fattr, NFDIR);
+      if (mp->am_child)
+	continue;
 
-    if ((--immediate_abort < 0 &&
-	 !(mp->am_flags & AMF_ROOT) && mp->am_parent) ||
-	(mf->mf_flags & MFF_RESTART)) {
+      mf = mp->am_al->al_mnt;
+      if (mf->mf_flags & MFF_UNMOUNTING) {
+	/*
+	 * If this node is being unmounted then just ignore it.  However,
+	 * this could prevent amd from finishing if the unmount gets blocked
+	 * since the am_node will never be free'd.  am_unmounted needs
+	 * telling about this possibility. - XXX
+	 */
+	continue;
+      }
+
+      if (!(mf->mf_fsflags & FS_DIRECTORY))
+	/*
+	 * When shutting down this had better
+	 * look like a directory, otherwise it
+	 * can't be unmounted!
+	 */
+	mk_fattr(&mp->am_fattr, NFDIR);
+
+      if ((--immediate_abort < 0 &&
+	   !(mp->am_flags & AMF_ROOT) && mp->am_parent) ||
+	  (mf->mf_flags & MFF_RESTART)) {
+
+	work_done++;
 
-      /*
-       * Just throw this node away without bothering to unmount it.  If
-       * the server is not known to be up then don't discard the mounted
-       * on directory or Amd might hang...
-       */
-      if (mf->mf_server &&
-	  (mf->mf_server->fs_flags & (FSF_DOWN | FSF_VALID)) != FSF_VALID)
-	mf->mf_flags &= ~MFF_MKMNT;
-      if (gopt.flags & CFM_UNMOUNT_ON_EXIT || mp->am_flags & AMF_AUTOFS) {
-	plog(XLOG_INFO, "on-exit attempt to unmount %s", mf->mf_mount);
 	/*
-	 * use unmount_mp, not unmount_node, so that unmounts be
-	 * backgrounded as needed.
+	 * Just throw this node away without bothering to unmount it.  If
+	 * the server is not known to be up then don't discard the mounted
+	 * on directory or Amd might hang...
 	 */
-	unmount_mp((opaque_t) mp);
+	if (mf->mf_server &&
+	    (mf->mf_server->fs_flags & (FSF_DOWN | FSF_VALID)) != FSF_VALID)
+	  mf->mf_flags &= ~MFF_MKMNT;
+	if (gopt.flags & CFM_UNMOUNT_ON_EXIT || mp->am_flags & AMF_AUTOFS) {
+	  plog(XLOG_INFO, "on-exit attempt to unmount %s", mf->mf_mount);
+	  /*
+	   * use unmount_mp, not unmount_node, so that unmounts be
+	   * backgrounded as needed.
+	   */
+	  unmount_mp((opaque_t) mp);
+	} else {
+	  am_unmounted(mp);
+	}
+	exported_ap[i] = NULL;
       } else {
-	am_unmounted(mp);
+	/*
+	 * Any other node gets forcibly timed out.
+	 */
+	mp->am_flags &= ~AMF_NOTIMEOUT;
+	mp->am_al->al_mnt->mf_flags &= ~MFF_RSTKEEP;
+	mp->am_ttl = 0;
+	mp->am_timeo = 1;
+	mp->am_timeo_w = 0;
       }
-      exported_ap[i] = NULL;
-    } else {
-      /*
-       * Any other node gets forcibly timed out.
-       */
-      mp->am_flags &= ~AMF_NOTIMEOUT;
-      mp->am_al->al_mnt->mf_flags &= ~MFF_RSTKEEP;
-      mp->am_ttl = 0;
-      mp->am_timeo = 1;
-      mp->am_timeo_w = 0;
     }
-  }
+  } while (work_done);
 }