Re: [RFC PATCH v8 01/14] mm: Add an allocator for CoCo shared memory

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mm: alloc_cc_shared_pages_node fails to strip __GFP_MOVABLE
- [Medium] mm: alloc_cc_shared_pages_node fails to strip __GFP_HIGHMEM in fallback path
- [Medium] mm: cc_make_shared and cc_make_private are not exported for modular callers
--

commit 3978c8ce72bbc2aa23e57a460b585808665262a4
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
mm: Add an allocator for CoCo shared memory

This commit introduces a common allocator for confidential-computing guests
to allocate shared memory. It provides helpers to allocate properly aligned
contiguous pages and transition them to shared state. It also adds helpers
for transitioning memory states when callers manage their own backing memory.

> diff --git a/mm/cc_shared.c b/mm/cc_shared.c
> new file mode 100644
> index 0000000000000..85e16f4504b8f
> --- /dev/null
> +++ b/mm/cc_shared.c
[ ... ]
> +int cc_make_shared(void *addr, size_t size)
> +{
> +	int ret = cc_validate_transition(addr, size);
> +
> +	if (ret)
> +		return ret;
> +
> +	return set_memory_decrypted((unsigned long)addr, size >> PAGE_SHIFT);
> +}

[Severity: Medium]
Will this function be unavailable to dynamically loadable modules since it
is missing an EXPORT_SYMBOL_GPL() macro?

The commit message states it should be usable by callers managing their own
backing memory, but virtio or guest drivers won't be able to link to it
without an export.

> +
> +int cc_make_private(void *addr, size_t size)
> +{
> +	int ret = cc_validate_transition(addr, size);
> +
> +	if (ret)
> +		return ret;
> +
> +	return set_memory_encrypted((unsigned long)addr, size >> PAGE_SHIFT);
> +}

[Severity: Medium]
Does this function also need an EXPORT_SYMBOL_GPL() macro so that modules
can successfully link and call it?

[ ... ]
> +static int __alloc_cc_shared_pages_node(int nid, gfp_t gfp,
> +					size_t requested,
> +					struct cc_shared_pages *mem)
> +{
[ ... ]
> +	/*
> +	 * 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);

[Severity: High]
Does this code inadvertently leave __GFP_MOVABLE intact if passed by the
caller?

Pages are returned as shared memory, but they lack migration hooks
(movable_operations) to handle encryption state transitions during page
migration.

If a caller passes __GFP_MOVABLE, the page might be placed in ZONE_MOVABLE.
Since the kernel migration code does not know how to handle encryption state
transitions, the page cannot be safely migrated. Could this permanently pin
the movable zone, thereby breaking memory hot-unplug and compaction?

> +	if (nid == NUMA_NO_NODE)
> +		page = alloc_pages(gfp, order);
[ ... ]
> +int alloc_cc_shared_pages_node(int nid, gfp_t gfp,
> +			       size_t requested,
> +			       struct cc_shared_pages *mem)
> +{
[ ... ]
> +	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> +		return __alloc_cc_shared_pages_node(nid, gfp, requested, mem);
> +
> +	order = get_order(requested);
> +	if (order > MAX_PAGE_ORDER)
> +		return -EINVAL;
> +
> +	if (nid == NUMA_NO_NODE)
> +		page = alloc_pages(gfp, order);
> +	else
> +		page = alloc_pages_node(nid, gfp, order);

[Severity: Medium]
Can this fallback path inadvertently allocate a highmem page?

The function's kernel-doc explicitly guarantees: "A memory-state transition
requires a valid linear-map address, so such allocations never come from
high memory". 

In the non-encrypted fallback path, the caller's unmodified gfp flags
(potentially containing __GFP_HIGHMEM) are passed directly to alloc_pages()
or alloc_pages_node() without any masking.

If a caller passes a flag mask containing __GFP_HIGHMEM (e.g., GFP_HIGHUSER)
on a system without memory encryption, the fallback path will allocate a
highmem page. Any subsequent attempt by the caller to access the memory via
page_address() could return NULL and cause a crash, breaking the API
contract. Should the gfp flags be sanitized in this path as well?

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