linux-kernel - [PATCH] nvme: fix handling mapping failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210119175336.4016923-1-marcorr@google.com>
Date:   Tue, 19 Jan 2021 09:53:36 -0800
From:   Marc Orr <marcorr@...gle.com>
To:     kbusch@...nel.org, axboe@...com, hch@....de, sagi@...mberg.me,
        jxgao@...gle.com, linux-nvme@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Cc:     Marc Orr <marcorr@...gle.com>, stable@...r.kernel.org
Subject: [PATCH] nvme: fix handling mapping failure

This patch ensures that when `nvme_map_data()` fails to map the
addresses in a scatter/gather list:

* The addresses are not incorrectly unmapped. The underlying
scatter/gather code unmaps the addresses after detecting a failure.
Thus, unmapping them again in the driver is a bug.
* The DMA pool allocations are not deallocated when they were never
allocated.

The bug that motivated this patch was the following sequence, which
occurred within the NVMe driver, with the kernel flag `swiotlb=force`.

* NVMe driver calls dma_direct_map_sg()
* dma_direct_map_sg() fails part way through the scatter gather/list
* dma_direct_map_sg() calls dma_direct_unmap_sg() to unmap any entries
  succeeded.
* NVMe driver calls dma_direct_unmap_sg(), redundantly, leading to a
  double unmap, which is a bug.

Before this patch, I observed intermittent application- and VM-level
failures when running a benchmark, fio, in an AMD SEV guest. This patch
resolves the failures.

Tested-by: Marc Orr <marcorr@...gle.com>
Cc: stable@...r.kernel.org
Signed-off-by: Marc Orr <marcorr@...gle.com>
---
 drivers/nvme/host/pci.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 9b1fc8633cfe..8b504ed08321 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -543,11 +543,14 @@ static void nvme_unmap_data(struct nvme_dev *dev, struct request *req)

 	WARN_ON_ONCE(!iod->nents);

-	if (is_pci_p2pdma_page(sg_page(iod->sg)))
-		pci_p2pdma_unmap_sg(dev->dev, iod->sg, iod->nents,
-				    rq_dma_dir(req));
-	else
-		dma_unmap_sg(dev->dev, iod->sg, iod->nents, rq_dma_dir(req));
+	if (!dma_mapping_error(dev->dev, iod->first_dma)) {
+		if (is_pci_p2pdma_page(sg_page(iod->sg)))
+			pci_p2pdma_unmap_sg(dev->dev, iod->sg, iod->nents,
+					    rq_dma_dir(req));
+		else
+			dma_unmap_sg(dev->dev, iod->sg, iod->nents,
+				     rq_dma_dir(req));
+	}

 	if (iod->npages == 0)
@@ -836,8 +839,11 @@ static blk_status_t nvme_map_data(struct nvme_dev *dev, struct request *req,
 	else
 		nr_mapped = dma_map_sg_attrs(dev->dev, iod->sg, iod->nents,
 					     rq_dma_dir(req), DMA_ATTR_NO_WARN);
-	if (!nr_mapped)
+	if (!nr_mapped) {
+		iod->first_dma = DMA_MAPPING_ERROR;
+		iod->npages = -1;
 		goto out;
+	}

 	iod->use_sgl = nvme_pci_use_sgls(dev, req);
 	if (iod->use_sgl)
-- 
2.30.0.284.gd98b1dd5eaa7-goog