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);