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

[Christian König]

On 8/20/26 12:50, Steven Price wrote:
> 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@xxxxxxx>
> ---
>  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));

To be honest that looks horrible.

If you need to backport that fine by me, but in the long term we clearly should have that in the initial allocation somehow.

Regards,
Christian.

>  		}
>  	}
>