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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] In `system_heap_allocate()`, if `set_memory_decrypted()` fails for an intermediate page, the error handling path attempts to re-encrypt all pages in the buffer, including those that were never decrypted. This can cause state-transition hypercalls to fail on already-encrypted pages, permanently leaking pristine memory.
--

--- Patch [2]: [PATCH 2/2] dma-buf: heaps: Zero system shared heap pages after conversion ---
commit 6c2e28c1010d31d528df19fa7fecbc4eb3e5bcad
Author: Steven Price <steven.price@arm.com>

dma-buf: heaps: Zero system shared heap pages after conversion

This commit modifies the system_cc_shared heap to defer zeroing pages until
after they have been successfully converted from private to shared. This
ensures that the guest clears the memory after decryption, as the decryption
process changes the visible data.

> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c

[ ... ]

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

[Severity: High]
This is a pre-existing issue, but does this error path permanently leak
pages if decryption fails midway?

If system_heap_set_page_decrypted() fails for an intermediate page, the
code jumps to free_pages. The free_pages label iterates over all pages in
the scatterlist unconditionally up to orig_nents:

free_pages:
        for_each_sgtable_sg(table, sg, i) {
                struct page *p = sg_page(sg);
                if (cc_shared_buffer(buffer) &&
                    system_heap_set_page_encrypted(p))
                        continue;
                __free_pages(p, compound_order(p));
        }

For pages later in the scatterlist that were never successfully decrypted,
system_heap_set_page_encrypted() will be called on already-encrypted memory.
If the hypervisor rejects this duplicate encryption request, the pristine
pages skip __free_pages() and are permanently leaked.

Is there a way to track exactly which pages were decrypted so the error path
only attempts to re-encrypt those specific pages?

> +
> +			clear_pages(page_address(page), 1 << compound_order(page));
>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820105026.53208-1-steven.price@arm.com?part=2