Re: [PATCH] swiotlb: check set_memory_decrypted()'s return value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022-11-21 19:48, Dexuan Cui wrote:
swiotlb_update_mem_attributes() is called from a system where decrypted
swiotlb bounce buffers are required. Panic the system if
set_memory_decrypted() fails.

When rmem_swiotlb_device_init() -> set_memory_decrypted(), let's try
to handle it gracefully.

Not sure how to handle the failure in swiotlb_init_late(). Add a WARN().

Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
---
  kernel/dma/swiotlb.c | 42 +++++++++++++++++++++++++++++-------------
  1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 339a990554e7..8e26c09c625c 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -248,12 +248,16 @@ void __init swiotlb_update_mem_attributes(void)
  	struct io_tlb_mem *mem = &io_tlb_default_mem;
  	void *vaddr;
  	unsigned long bytes;
+	int rc;
if (!mem->nslabs || mem->late_alloc)
  		return;
  	vaddr = phys_to_virt(mem->start);
  	bytes = PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT);
-	set_memory_decrypted((unsigned long)vaddr, bytes >> PAGE_SHIFT);
+
+	rc = set_memory_decrypted((unsigned long)vaddr, bytes >> PAGE_SHIFT);
+	if (rc)
+		panic("Failed to decrypt swiotlb bounce buffers (%d)\n", rc);

Aww, I just cleaned up all the panics! AFAICS we could warn and set mem->nslabs to 0 here to semi-gracefully disable SWIOTLB (presumably decryption failure is sufficiently unexpected that "leaking" the SWIOTLB memory is the least of the system's problems). Or indeed just warn and do nothing as in the swiotlb_init_late() case below - what's with the inconsistency? In either path we have the same expectation that decryption succeeds (or does nothing, successfully), so failure is no more or less fatal to later SWIOTLB operation depending on when the architecture chose to set it up.

  	mem->vaddr = swiotlb_mem_remap(mem, bytes);
  	if (!mem->vaddr)
@@ -447,8 +451,10 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
  	if (!mem->slots)
  		goto error_slots;
- set_memory_decrypted((unsigned long)vstart,
-			     (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
+	rc = set_memory_decrypted((unsigned long)vstart,
+				  (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
+	WARN(rc, "Failed to decrypt swiotlb bounce buffers (%d)\n", rc);
+
  	swiotlb_init_io_tlb_mem(mem, virt_to_phys(vstart), nslabs, 0, true,
  				default_nareas);
@@ -986,6 +992,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem, /* Set Per-device io tlb area to one */
  	unsigned int nareas = 1;
+	int rc = -ENOMEM;
/*
  	 * Since multiple devices can share the same pool, the private data,
@@ -998,21 +1005,22 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
  			return -ENOMEM;
mem->slots = kcalloc(nslabs, sizeof(*mem->slots), GFP_KERNEL);
-		if (!mem->slots) {
-			kfree(mem);
-			return -ENOMEM;
-		}
+		if (!mem->slots)
+			goto free_mem;
mem->areas = kcalloc(nareas, sizeof(*mem->areas),
  				GFP_KERNEL);
-		if (!mem->areas) {
-			kfree(mem->slots);
-			kfree(mem);
-			return -ENOMEM;
+		if (!mem->areas)
+			goto free_slots;
+
+		rc = set_memory_decrypted(
+				(unsigned long)phys_to_virt(rmem->base),
+				rmem->size >> PAGE_SHIFT);
+		if (rc) {
+			pr_err("Failed to decrypt rmem buffer (%d)\n", rc);
+			goto free_areas;

So in 3 init paths we have 3 different outcomes from the same thing :(

1: make the whole system unusable
2: continue with possible data corruption (or at least weird DMA errors) if devices still see encrypted memory contents
3: cleanly disable SWIOTLB, thus also all subsequent attempts to use it

(OK, for the rmem case this isn't actually 3 since falling back to io_tlb_default_mem might work out more like 2, but hopefully you get my point)

Thanks,
Robin.

  		}
- set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
-				     rmem->size >> PAGE_SHIFT);
  		swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, SWIOTLB_FORCE,
  					false, nareas);
  		mem->for_alloc = true;
@@ -1025,6 +1033,14 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
  	dev->dma_io_tlb_mem = mem;
return 0;
+
+free_areas:
+	kfree(mem->areas);
+free_slots:
+	kfree(mem->slots);
+free_mem:
+	kfree(mem);
+	return rc;
  }
static void rmem_swiotlb_device_release(struct reserved_mem *rmem,



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux