Re: [PATCH v9 06/14] dma-pool: Allocate CoCo atomic pools using CoCo shared memory allocator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dma-pool: cc_make_shared() runs after arch_dma_prep_coherent()
--

commit daae4fcc15a00bc83c4081ef4b27786a7f224537
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

dma-pool: Allocate CoCo atomic pools using CoCo shared memory allocator

This commit updates atomic_pool_expand() to allocate confidential computing
shared atomic DMA pools using the cc_shared layout and transition helpers
(cc_shared_calc_layout(), cc_make_shared(), and cc_make_private()), enforcing
shared granule size alignment and minimum allocation order.

> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index 70b7f64b17ab5..da487c3f9dc61 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c

[ ... ]

> @@ -126,8 +142,7 @@ static int atomic_pool_expand(struct dma_gen_pool *dma_pool, size_t pool_size,
>  	 * shrink so no re-encryption occurs in dma_direct_free().
>  	 */
>  	if (dma_pool->cc_shared) {
> -		ret = set_memory_decrypted((unsigned long)page_to_virt(page),
> -					   1 << order);
> +		ret = cc_make_shared(page_to_virt(page), pool_size);

[Severity: High]
Does calling cc_make_shared(page_to_virt(page), pool_size) after
arch_dma_prep_coherent() and dma_common_contiguous_remap() leave dirty cache
lines on the cacheable linear mapping alias when CONFIG_DMA_DIRECT_REMAP is
enabled?

In atomic_pool_expand(), arch_dma_prep_coherent() and the vmap remapping run
first:

kernel/dma/pool.c:atomic_pool_expand() {
	...
	arch_dma_prep_coherent(page, pool_size);

#ifdef CONFIG_DMA_DIRECT_REMAP
	if (dma_pool->cc_shared)
		prot = pgprot_decrypted(pgprot_dmacoherent(PAGE_KERNEL));
	else
		prot = pgprot_dmacoherent(PAGE_KERNEL);

	addr = dma_common_contiguous_remap(page, pool_size, prot,
			__builtin_return_address(0));
	if (!addr)
		goto free_page;
#else
	addr = page_to_virt(page);
#endif
	if (dma_pool->cc_shared) {
		ret = cc_make_shared(page_to_virt(page), pool_size);
	...
}

Because set_memory_decrypted() (in commit 0a47431deefc ("mm: Zero memory
during shared memory transitions")) zeroes the range through the cacheable
linear map address passed to it:

cc_make_shared()
  -> set_memory_decrypted()
       -> realm_set_memory_decrypted() / pkvm_set_memory_decrypted()
            -> memset((void *)addr, 0, (size_t)numpages << PAGE_SHIFT);

performing this transition after arch_dma_prep_coherent() has already cleaned
the linear map alias dirties the CPU data cache for page_to_virt(page) with
zeros without a subsequent cache clean (and on Arm CCA, cleans the protected
physical address alias before the RIPAS transition rather than the shared
physical address alias after zeroing). Those dirty cache lines on the linear
mapping can later be evicted to the Point of Coherency and overwrite live DMA
buffers accessed via the non-cacheable vmap alias at addr.

In contrast, dma_direct_alloc() transitions the pages before calling
arch_dma_prep_coherent() and dma_common_contiguous_remap(). Should
atomic_pool_expand() similarly call cc_make_shared() before
arch_dma_prep_coherent() and dma_common_contiguous_remap()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104164555.3555725-1-aneesh.kumar@kernel.org?part=6