⬢ DragonFlyBSD Kernel Audit
DF-4607 / 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
@@ -305,6 +305,30 @@ static void
 ata_completed(void *context, int dummy)
 {
     struct ata_request *request = (struct ata_request *)context;
     struct ata_channel *ch = device_get_softc(request->parent);
     struct ata_device *atadev = device_get_softc(request->dev);
     struct ata_composite *composite;
 
+    /*
+     * DF-4607: the 4x-timeout zombie recovery (ata_queue_request) calls
+     * ata_completed() directly and concurrently with the taskqueue
+     * completion that may finally have arrived.  The DANGER1/DANGER2
+     * transitions below were unlocked, so both threads could pass the
+     * checks and run the completion body twice, and the zombie free
+     * could land while the direct caller was still inside the body
+     * (UAF), after which the submitter's own ata_free_request() puts the
+     * same object back into the cache a second time (double free).
+     *
+     * Serialize the transitions on the request's done lock (still
+     * initialized for !callback requests, which is exactly the DANGER
+     * case) and make completion idempotent for non-callback requests.
+     */
+    if (!request->callback) {
+	lockmgr(&request->done, LK_EXCLUSIVE);
+	if (request->flags & ATA_R_COMPLETED) {
+	    lockmgr(&request->done, LK_RELEASE);
+	    return;
+	}
+	if (request->flags & ATA_R_DANGER2) {
+	    device_printf(request->dev,
+			  "WARNING - %s freeing taskqueue zombie request\n",
+			  ata_cmd2str(request));
+	    request->flags &= ~(ATA_R_DANGER1 | ATA_R_DANGER2);
+	    lockmgr(&request->done, LK_RELEASE);
+	    ata_free_request(request);
+	    return;
+	}
+	if (request->flags & ATA_R_DANGER1)
+	    request->flags |= ATA_R_DANGER2;
+	lockmgr(&request->done, LK_RELEASE);
+    }
+
     if (request->flags & ATA_R_DANGER2) {
 	device_printf(request->dev,
 		      "WARNING - %s freeing taskqueue zombie request\n",
 		      ata_cmd2str(request));
 	request->flags &= ~(ATA_R_DANGER1 | ATA_R_DANGER2);