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 03:58:25PM +0530, Aneesh Kumar K.V wrote:
> Catalin Marinas <catalin.marinas@arm.com> writes:
> > 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.
> >
> 
> I checked this, and you are right. We cannot expect the contents to
> remain valid across sharing; set_memory_decrypted() is destructive in
> that sense. Since pKVM does not rely on memory encryption, it needs to
> zero the memory unconditionally in set_memory_decrypted() to avoid
> exposing existing guest data. Other CoCo implementations may omit the
> memset(0). We still need to zero the memory when the caller requests
> __GFP_ZERO, where the memory location is expected to be zero.
> 
> We could either zero the memory unconditionally or pass a flag to allow
> this micro-optimization. We would also need to audit all call paths to
> avoid redundant zeroing after set_memory_decrypted(). Let me know if you
> have a preference for either approach.

The simplest is probably to always zero in the backend and ignore
__GFP_ZERO to the allocator. But it's probably only marginally smaller
than passing a CC_SHARED_ZERO flag down. Get codex to try this as well
and compare the diffstat.

There's an argument for the flag approach from a performance perspective
(avoid zeroing unnecessarily) but not sure how much it matters in
practice.

-- 
Catalin