On Wed, Sep 23, 2026 at 03:58:25PM +0530, Aneesh Kumar K.V wrote: > Catalin Marinas <[email protected]> writes: > > On Wed, Sep 23, 2026 at 11:23:27AM +0530, Aneesh Kumar K.V wrote: > >> Catalin Marinas <[email protected]> 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
