5360 Race condition in devfs upgrades reader to writer incidentally and causes panic

@@ -18,10 +18,11 @@
  *
  * CDDL HEADER END
  */
 
 /*
+ * Copyright 2014 Coraid, Inc.
  * Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
  */
 
 /*
  * This file implements /dev filesystem operations for non-global

@@ -653,55 +654,89 @@
                     name, name);
         }
 }
 
 /*
- * Build directory vnodes based on the profile and the global
- * dev instance.
+ * Return True if directory cache is out of date and should be updated.
  */
-void
-prof_filldir(struct sdev_node *ddv)
+static boolean_t
+prof_dev_needupdate(sdev_node_t *ddv)
 {
-        int firsttime = 1;
-        struct sdev_node *gdir = ddv->sdev_origin;
+        sdev_node_t *gdir = ddv->sdev_origin;
 
-        ASSERT(RW_READ_HELD(&ddv->sdev_contents));
+        /*
+         * Caller can have either reader or writer lock
+         */
+        ASSERT(RW_LOCK_HELD(&ddv->sdev_contents));
 
         /*
          * We need to rebuild the directory content if
-         * - SDEV_BUILD is set
-         * - The device tree generation number has changed
+         * - ddv is not in a SDEV_ZOMBIE state
+         * - SDEV_BUILD is set OR
+         * - The device tree generation number has changed OR
          * - The corresponding /dev namespace has been updated
          */
-check_build:
-        if ((ddv->sdev_flags & SDEV_BUILD) == 0 &&
-            ddv->sdev_devtree_gen == devtree_gen &&
-            (gdir == NULL || ddv->sdev_ldir_gen
-            == gdir->sdev_gdir_gen))
-                return;         /* already up to date */
+        return ((ddv->sdev_state != SDEV_ZOMBIE) &&
+            (((ddv->sdev_flags & SDEV_BUILD) != 0) ||
+            (ddv->sdev_devtree_gen != devtree_gen) ||
+            ((gdir != NULL) &&
+            (ddv->sdev_ldir_gen != gdir->sdev_gdir_gen))));
+}
 
-        /* We may have become a zombie (across a try) */
-        if (ddv->sdev_state == SDEV_ZOMBIE)
-                return;
+/*
+ * Build directory vnodes based on the profile and the global
+ * dev instance.
+ */
+void
+prof_filldir(sdev_node_t *ddv)
+{
+        sdev_node_t *gdir;
 
-        if (firsttime && rw_tryupgrade(&ddv->sdev_contents) == 0) {
+        ASSERT(RW_READ_HELD(&ddv->sdev_contents));
+
+        if (!prof_dev_needupdate(ddv)) {
+                ASSERT(RW_READ_HELD(&ddv->sdev_contents));
+                return;
+        }
+        /*
+         * Upgrade to writer lock
+         */
+        if (rw_tryupgrade(&ddv->sdev_contents) == 0) {
+                /*
+                 * We need to drop the read lock and re-acquire it as a
+                 * write lock. While we do this the condition may change so we
+                 * need to re-check condition.
+                 * NOTE: if device becomes a zombie, prof_dev_needupdate returns
+                 * false, so nothing we will just return in this case.
+                 */
                 rw_exit(&ddv->sdev_contents);
-                firsttime = 0;
                 rw_enter(&ddv->sdev_contents, RW_WRITER);
-                goto check_build;
+                if (!prof_dev_needupdate(ddv)) {
+                        /* Downgrade back to the read lock before returning */
+                        rw_downgrade(&ddv->sdev_contents);
+                        return;
         }
+        }
+        /* At this point we should have a write lock */
+        ASSERT(RW_WRITE_HELD(&ddv->sdev_contents));
+
         sdcmn_err10(("devtree_gen (%s): %ld -> %ld\n",
             ddv->sdev_path, ddv->sdev_devtree_gen, devtree_gen));
-        if (gdir)
+
+        gdir = ddv->sdev_origin;
+
+        if (gdir != NULL)
                 sdcmn_err10(("sdev_dir_gen (%s): %ld -> %ld\n",
                     ddv->sdev_path, ddv->sdev_ldir_gen,
                     gdir->sdev_gdir_gen));
 
         /* update flags and generation number so next filldir is quick */
+        if ((ddv->sdev_flags & SDEV_BUILD) == SDEV_BUILD) {
         ddv->sdev_flags &= ~SDEV_BUILD;
+        }
         ddv->sdev_devtree_gen = devtree_gen;
-        if (gdir)
+        if (gdir != NULL)
                 ddv->sdev_ldir_gen = gdir->sdev_gdir_gen;
 
         prof_make_symlinks(ddv);
         prof_make_maps(ddv);
         prof_make_names(ddv);