⬢ DragonFlyBSD Kernel Audit
DF-4606 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/dev/disk/nata/ata-queue.c b/sys/dev/disk/nata/ata-queue.c
index 1111111..2222222 100644
--- a/sys/dev/disk/nata/ata-queue.c
+++ b/sys/dev/disk/nata/ata-queue.c
@@ -549,12 +549,24 @@ void
 ata_timeout(struct ata_request *request)
 {
     struct ata_channel *ch = device_get_softc(request->parent);
 
     /*
      * NOTE: callout acquired ch->state_mtx for us.
      */
     /*request->flags |= ATA_R_DEBUG;*/
     ATA_DEBUG_RQ(request, "timeout");
 
     /*
      * if we have an ATA_ACTIVE request running, we flag the request 
      * ATA_R_TIMEOUT so ata_finish will handle it correctly
      * also NULL out the running request so we wont loose 
      * the race with an eventual interrupt arriving late
      */
     if (ch->state == ATA_ACTIVE) {
 	request->flags |= ATA_R_TIMEOUT;
+	/*
+	 * DF-4606: NULL out ch->running while we still hold state_mtx --
+	 * the comment above claims this is done, but the code never did it.
+	 * Leaving ch->running set opens a window between the LK_RELEASE
+	 * below and the LK_EXCLUSIVE at the bottom of this function in
+	 * which a late ata_interrupt() (ata-all.c:319-363), ata_reinit()
+	 * (ata-all.c:206-208) or ata_fail_requests() (ata-queue.c:577-582)
+	 * can take state_mtx and reach callout_cancel(&request->callout)
+	 * while this callback is INPROG.  _callout_cancel_or_stop(sync=1)
+	 * then ssleep()s holding state_mtx waiting for this callback to
+	 * return, while this callback blocks re-acquiring state_mtx:
+	 * permanent deadlock (softclock thread + channel lock lost).
+	 * The same window also lets a late interrupt complete the request
+	 * a second time (double ata_finish / double ATA_LF_UNLOCK).
+	 */
+	ch->running = NULL;
 	lockmgr(&ch->state_mtx, LK_RELEASE);
 	ATA_LOCKING(ch->dev, ATA_LF_UNLOCK);
 	ata_finish(request);
 	lockmgr(&ch->state_mtx, LK_EXCLUSIVE);
     }
 }