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

Pre-existing issues:
- [High] Unconditional page re-encryption in error path causes potential memory 
leak and invalid hypercalls
--

commit 611c6660a2bb2f9f324ce349cfce6b2530846d28
Author: Arnd Bergmann <[email protected]>

dma-buf: move system_cc_shared heap under separate Kconfig

This commit moves the system_cc_shared heap into its own Kconfig symbol,
DMABUF_HEAPS_CC_SYSTEM. This resolves a build failure on architectures like
powerpc and s390 when the system heap is built as a loadable module, since
the set_memory_encrypted and set_memory_decrypted functions are not
exported to modules.

> diff --git a/drivers/dma-buf/heaps/system_heap.c 
> b/drivers/dma-buf/heaps/system_heap.c
> index c92bdec356fcc..71d9028cc3dfc 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
[ ... ]
> @@ -485,7 +488,7 @@ static struct dma_buf *system_heap_allocate(struct 
> dma_heap *heap,
>                * Intentionally leak pages that cannot be re-encrypted
>                * to prevent shared memory from being reused.
>                */
> -             if (buffer->cc_shared &&
> +             if (cc_shared_buffer(buffer) &&
>                   system_heap_set_page_encrypted(p))
>                       continue;
>               __free_pages(p, compound_order(p));

[Severity: High]
This isn't a bug introduced by this patch, but does this error path
unintentionally re-encrypt pages that were never decrypted?

If system_heap_set_page_decrypted() fails during the earlier loop in
system_heap_allocate(), it jumps to the free_pages label. However, the
cleanup loop here uses for_each_sgtable_sg(table, sg, i), which iterates
over the entire scatterlist starting from index 0.

This means pages after the failure point, which were never decrypted, will
still be passed to system_heap_set_page_encrypted(). If the underlying
set_memory_encrypted() call fails on already-encrypted pages (e.g., due to
hypervisor state enforcement), the code skips __free_pages() and leaks the
memory.

Could this loop be modified to only re-encrypt the pages that were
successfully decrypted before the failure?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to