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 6ec3e48cd1b649d6f45e50018432fb94ce422a5a..0379aadf69fb1902a30b2b96007869903eab0187 100644 (file)
--- 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 ac707cc7bf1109047444c420bd75d88f495c8a36..0b06f0d91b10f9cef22fefe4236ebd0777d80713 100644 (file)
--- 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);
 }