CAM - Fix missing SIM lock in cam_periph_release()

* When releasing the last reference on a CAM peripheral which has been
  marked invalid, the peripheral is then freed.  Freeing the peripheral
  requires the SIM lock.

  Go through necessary tribulations to acquire the SIM lock.  The lock order
  is important (SIM lock first, XPT lock second), and the SIM lock may or
  may not already be held by the thread.

* The bug could cause a crash after burning a CD as the CD becomes invalid
  after the burn completes in order to reload its state.

diff --git a/sys/bus/cam/cam_periph.c b/sys/bus/cam/cam_periph.c
index 988edec..839902c 100644 (file)
--- a/sys/bus/cam/cam_periph.c
+++ b/sys/bus/cam/cam_periph.c
@@ -284,20 +284,54 @@ cam_periph_acquire(struct cam_periph *periph)
        return(CAM_REQ_CMP);
 }
 
+/*
+ * Release the peripheral.  The XPT is not locked and the SIM may or may
+ * not be locked on entry.
+ *
+ * The last release on a peripheral marked invalid frees it.  In this
+ * case we must be sure to hold both the XPT lock and the SIM lock,
+ * requiring a bit of fancy footwork if the SIM lock already happens
+ * to be held.
+ */
 void
 cam_periph_release(struct cam_periph *periph)
 {
+       struct cam_sim *sim;
+       int doun;
 
-       if (periph == NULL)
-               return;
+       while (periph) {
+               /*
+                * First try the critical path case
+                */
+               sim = periph->sim;
+               xpt_lock_buses();
+               if ((periph->flags & CAM_PERIPH_INVALID) == 0 ||
+                   periph->refcount != 1) {
+                       --periph->refcount;
+                       xpt_unlock_buses();
+                       break;
+               }
 
-       xpt_lock_buses();
-       if ((--periph->refcount == 0)
-        && (periph->flags & CAM_PERIPH_INVALID)) {
-               camperiphfree(periph);
+               /*
+                * Otherwise we also need to free the peripheral and must
+                * acquire the sim lock and xpt lock in the correct order
+                * to do so.
+                *
+                * The condition must be re-checked after the locks have
+                * been reacquired.
+                */
+               xpt_unlock_buses();
+               doun = CAM_SIM_COND_LOCK(sim);
+               xpt_lock_buses();
+               --periph->refcount;
+               if ((periph->flags & CAM_PERIPH_INVALID) &&
+                   periph->refcount == 0) {
+                       camperiphfree(periph);
+               }
+               xpt_unlock_buses();
+               CAM_SIM_COND_UNLOCK(sim, doun);
+               break;
        }
-       xpt_unlock_buses();
-
 }
 
 int

diff --git a/sys/bus/cam/cam_sim.c b/sys/bus/cam/cam_sim.c
index fcb770d..be18d29 100644 (file)
--- a/sys/bus/cam/cam_sim.c
+++ b/sys/bus/cam/cam_sim.c
@@ -69,6 +69,30 @@ cam_sim_unlock(sim_lock *lock)
                lockmgr(lock, LK_RELEASE);
 }
 
+int
+cam_sim_cond_lock(sim_lock *lock)
+{
+       if (lock == &sim_mplock) {
+               get_mplock();
+               return(1);
+       } else if (lockstatus(lock, curthread) != LK_EXCLUSIVE) {
+               lockmgr(lock, LK_EXCLUSIVE);
+               return(1);
+       }
+       return(0);
+}
+
+void
+cam_sim_cond_unlock(sim_lock *lock, int doun)
+{
+       if (doun) {
+               if (lock == &sim_mplock)
+                       rel_mplock();
+               else
+                       lockmgr(lock, LK_RELEASE);
+       }
+}
+
 /*
  * lock can be NULL if sim was &dead_sim
  */

diff --git a/sys/bus/cam/cam_sim.h b/sys/bus/cam/cam_sim.h
index c1290d4..761d963 100644 (file)
--- a/sys/bus/cam/cam_sim.h
+++ b/sys/bus/cam/cam_sim.h
@@ -58,6 +58,8 @@ extern        sim_lock        sim_mplock;
 
 void             cam_sim_lock(sim_lock *lock);
 void             cam_sim_unlock(sim_lock *lock);
+int              cam_sim_cond_lock(sim_lock *lock);
+void             cam_sim_cond_unlock(sim_lock *lock, int doun);
 void             sim_lock_assert_owned(sim_lock *lock);
 void             sim_lock_assert_unowned(sim_lock *lock);
 int              sim_lock_sleep(void *ident, int flags, const char *wmesg,
@@ -138,8 +140,10 @@ struct cam_sim {
 
 };
 
-#define CAM_SIM_LOCK(sim)      cam_sim_lock((sim)->lock);
-#define CAM_SIM_UNLOCK(sim)    cam_sim_unlock((sim)->lock);
+#define CAM_SIM_LOCK(sim)              cam_sim_lock((sim)->lock)
+#define CAM_SIM_UNLOCK(sim)            cam_sim_unlock((sim)->lock)
+#define CAM_SIM_COND_LOCK(sim)         cam_sim_cond_lock((sim)->lock)
+#define CAM_SIM_COND_UNLOCK(sim, doun) cam_sim_cond_unlock((sim)->lock, doun)
 
 static __inline u_int32_t
 cam_sim_path(struct cam_sim *sim)