[PATCH 0/2] Clear shared pages after private-to-shared conversion

[PATCH 0/2] Clear shared pages after private-to-shared conversion

[Steven Price]

Arm CCA includes "Memory Encryption Contexts" (MEC) which allows the
private and shared data accessible to a guest to have different memory
encryption keys. Consequently when converting memory to shared, the
memory encryption key used to access the physical page will change.

Both the GICv3 ITS driver and the system_cc_shared dma-buf heap
currently allocate memory with __GFP_ZERO and then decrypt it. With MEC
the zeroing is done with the wrong encryption key and the data visible
after decryption may be ciphertext. The RMM is required to scrub the
data, but may perform this scrub with a different encryption key to the
eventual key that will be used for shared access.

Fix these two sites by avoiding the __GFP_ZERO during the allocation and
performing a clear_pages() call after the decryption.

Steven Price (2):
  irqchip/gic-v3-its: Zero shared pages after conversion
  dma-buf: heaps: Zero system shared heap pages after conversion

 drivers/dma-buf/heaps/system_heap.c | 14 +++++++++++---
 drivers/irqchip/irq-gic-v3-its.c    |  7 ++++++-
 2 files changed, 17 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH 1/2] irqchip/gic-v3-its: Zero shared pages after conversion

[Steven Price]

its_alloc_pages_node() passes __GFP_ZERO to the page allocator before
calling set_memory_decrypted(). This assumes that converting a page from
private to shared preserves its contents.

For Arm CCA with MEC (Memory Encryption Contexts) the key used to access
the page will change, and so by default the visible data will change.
The host could ensure that it zeros the page, but rather than relying on
the host's behaviour it's best if the guest simply zeros after the
decryption rather than before. Specifically in this case the ITS tables
are required to be zeroed.

Mask out __GFP_ZERO from the allocation request, and do the zeroing as a
separate step after decryption.

Fixes: b08e2f42e86b ("irqchip/gic-v3-its: Share ITS tables with a non-trusted hypervisor")
Signed-off-by: Steven Price <steven.price@arm.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 6f5811aae59c..c954bbe9f4db 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -213,10 +213,12 @@ static gfp_t gfp_flags_quirk;
 static struct page *its_alloc_pages_node(int node, gfp_t gfp,
 					 unsigned int order)
 {
+	bool want_zero = gfp & __GFP_ZERO;
 	struct page *page;
 	int ret = 0;
 
-	page = alloc_pages_node(node, gfp | gfp_flags_quirk, order);
+	page = alloc_pages_node(node, (gfp & ~__GFP_ZERO) | gfp_flags_quirk,
+				order);
 
 	if (!page)
 		return NULL;
@@ -231,6 +233,9 @@ static struct page *its_alloc_pages_node(int node, gfp_t gfp,
 	if (ret)
 		return NULL;
 
+	if (want_zero)
+		clear_pages(page_address(page), 1 << order);
+
 	return page;
 }
 
-- 
2.43.0

[PATCH 2/2] dma-buf: heaps: Zero system shared heap pages after conversion

[Steven Price]

The system_cc_shared heap allocates pages with __GFP_ZERO before converting
them from private to shared with set_memory_decrypted(). This assumes that
the conversion preserves the contents of the pages.

For Arm CCA with MEC (Memory Encryption Contexts) the key used to access
the page will change, and so by default the visible data will change.
The host could ensure that it zeros the page after decryption, but
rather than relying on the host's behaviour it's best if the guest
simply zeros after the decryption rather than before.

For CC shared buffers, defer zeroing until each page has been converted
successfully. For other buffers keep the existing behaviour.

Fixes: 78b30c50a7ac ("dma-buf: heaps: system: add system_cc_shared heap for explicitly shared memory")
Signed-off-by: Steven Price <steven.price@arm.com>
---
 drivers/dma-buf/heaps/system_heap.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index c8959eadc71d..f14930904089 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -376,7 +376,8 @@ static const struct dma_buf_ops system_heap_buf_ops = {
 };
 
 static struct page *alloc_largest_available(unsigned long size,
-					    unsigned int max_order)
+					    unsigned int max_order,
+					    bool defer_zero)
 {
 	struct page *page;
 	int i;
@@ -388,6 +389,9 @@ static struct page *alloc_largest_available(unsigned long size,
 		if (max_order < orders[i])
 			continue;
 		flags = order_flags[i];
+		/* Decryption can change the contents, so clear it afterwards. */
+		if (defer_zero)
+			flags &= ~__GFP_ZERO;
 		if (mem_accounting)
 			flags |= __GFP_ACCOUNT;
 		page = alloc_pages(flags, orders[i]);
@@ -438,7 +442,8 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 			goto free_buffer;
 		}
 
-		page = alloc_largest_available(size_remaining, max_order);
+		page = alloc_largest_available(size_remaining, max_order,
+					       cc_shared_buffer(buffer));
 		if (!page)
 			goto free_buffer;
 
@@ -461,9 +466,12 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 
 	if (cc_shared_buffer(buffer)) {
 		for_each_sgtable_sg(table, sg, i) {
-			ret = system_heap_set_page_decrypted(sg_page(sg));
+			page = sg_page(sg);
+			ret = system_heap_set_page_decrypted(page);
 			if (ret)
 				goto free_pages;
+
+			clear_pages(page_address(page), 1 << compound_order(page));
 		}
 	}
 
-- 
2.43.0