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 <[email protected]> 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/[email protected]?part=2
