Re: [RFC PATCH v7 02/13] mm: Add an allocator for CoCo shared memory

[Catalin Marinas <catalin.marinas@arm.com>]

On Wed, Sep 23, 2026 at 11:23:27AM +0530, Aneesh Kumar K.V wrote:
> Catalin Marinas <catalin.marinas@arm.com> writes:
> > On Mon, Sep 21, 2026 at 08:18:36PM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> +int alloc_cc_shared_pages_node(int nid, gfp_t gfp,
> >> +		size_t requested, struct cc_shared_pages *mem)
> >> +{
> >> +	struct cc_shared_layout layout;
> >> +	struct page *page;
> >> +	unsigned int order;
> >> +	bool zero = gfp & __GFP_ZERO;
> >> +	int ret;
> >> +
> >> +	if (!mem)
> >> +		return -EINVAL;
> >> +
> >> +	ret = cc_shared_calc_layout(requested, &layout);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	order = get_order(layout.shared_size);
> >> +	if (order > MAX_PAGE_ORDER)
> >> +		return -EINVAL;
> >> +
> >> +	/*
> >> +	 * State transitions require a linear-map address and may modify memory.
> >> +	 * Allocate from low memory and defer requested zeroing until afterwards.
> >> +	 */
> >> +	gfp &= ~(__GFP_HIGHMEM | __GFP_ZERO);
> >> +	if (nid == NUMA_NO_NODE)
> >> +		page = alloc_pages(gfp, order);
> >> +	else
> >> +		page = alloc_pages_node(nid, gfp, order);
> >> +	if (!page)
> >> +		return -ENOMEM;
> >> +
> >> +	ret = cc_make_shared(page_address(page), layout.shared_size);
> >> +	if (ret) {
> >> +		if (!cc_make_private(page_address(page), layout.shared_size))
> >> +			__free_pages(page, order);
> >> +		else
> >> +			pr_warn_ratelimited("leaking %zu bytes with uncertain shared state\n",
> >> +					    layout.shared_size);
> >> +		return ret;
> >> +	}
> >> +
> >> +	if (zero)
> >> +		memset(page_address(page), 0, layout.shared_size);
> >
> > Does the memset() post sharing logic work for pKVM as well? If nothing
> > clears it, we have a small window where guest data is leaked to the
> > host.
> >
> > Is there a case where we *do not* need the memory cleared? If not, maybe
> > we can move the logic in the arch set_memory_decrypted().
> >
> 
> I don't think every architecture or platform can unconditionally zero
> memory in set_memory_decrypted(). Some callers may need to share valid
> contents with the host.

Is there any? That would be a bad assumptions in the caller. Most
set_memory_* backends don't preserve the content as they change the
encryption key. So properly written code shouldn't rely on this unless
it knows specifically it's only running on pKVM for example. The only
use-case I see to avoid explicit zeroing is when the caller doesn't care
about the page initialisation and wants to save some cycles. The
encryption key change would take care of the security aspect.

> Also, if zeroing is added only to the CCA implementation, the allocator
> must retain __GFP_ZERO for platforms such as pKVM. This would cause the
> memory to be zeroed twice on CCA.

What I meant is that we change the set_memory_decrypted() contract to
always zero, assuming that all callers need to zero the pages anyway. If
we do have cases where zeroing is not needed, we could make it explicit
via a flag.

> How about extending cc_make_shared() with a flag indicating that the
> memory must be zeroed, and passing that requirement down to the
> architecture-specific implementation? The implementation could then zero
> the memory at the appropriate point: before sharing for pKVM and after
> the destructive transition for CCA.

On pKVM, we want set_memory_decrypted() to zero the buffer
before the host can access it (I guess currently relying on __GFP_ZERO
allocations). Since no cryptographic encryption takes place, there's not
much point in memset'ing again after the operation as the content was
already zeroed.

I don't think cc_make_shared() has the right information on how to
safely and efficiently do the zeroing. That's only known to the
set_memory_* backend. So you'd have to propagate the flag down.

-- 
Catalin