[PATCH RFC] KVM: arm64: Document that overlapping MEM_SHARE and MEM_UNSHARE are unordered

[PATCH RFC] KVM: arm64: Document that overlapping MEM_SHARE and MEM_UNSHARE are unordered

[Xuheng Li]

The pKVM guest interface does not say what happens when a protected
guest issues MEM_SHARE and MEM_UNSHARE for the same page concurrently,
for example on two vCPUs.  The two calls are not ordered: a MEM_SHARE
that has not returned yet may take effect after a MEM_UNSHARE of the
same page has returned, and leave the page shared although the vCPU
that unshared it saw SUCCESS.

  vCPU A                         vCPU B
  ------                         ------
  MEM_SHARE(P) issued ...
                                 MEM_SHARE(P)    -> SUCCESS
                                 MEM_UNSHARE(P)  -> SUCCESS
                                 write private data to P
  ... MEM_SHARE(P) takes effect  -> SUCCESS: P is shared again

How long a share can stay pending is up to the host.  When the page is
not yet mapped in the guest's stage-2, pkvm_memshare_call() turns the
call into a data abort for the host (__pkvm_memshare_page_req()),
rewinds the guest's PC and exits; the share is performed from scratch
only when the host runs the vCPU again, with no record of the suspended
call and no lock held across it.  So the host can make vCPU A's share
complete after vCPU B's unshare, and read what B wrote to P.

Document that such calls are unordered, so that a guest knows it must
wait for all its shares of a region to return before treating the
region as private.

Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
We noticed this while studying pKVM's guest memory-sharing interface as
part of our research project, and are not sure whether documenting it is
the right answer.

A guest racing MEM_SHARE and MEM_UNSHARE of the same page on two vCPUs
is arguably asking for trouble, and Linux guests don't do it today.  But
nothing in the interface tells a guest when a page is private again,
and because a share of an unmapped page is suspended until the host
runs the vCPU again, the host controls how long the window stays open.
The alternative to this documentation change would be to record a
suspended MEM_SHARE (for example in the hyp vCPU) so that a later
MEM_UNSHARE of the same page cancels it, or to fail the unshare while a
share of that page is pending.  We'd be happy to work on either if you
prefer a code change.
---
 Documentation/virt/kvm/arm/hypercalls.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/virt/kvm/arm/hypercalls.rst b/Documentation/virt/kvm/arm/hypercalls.rst
index 7400a89aa..ad7082da1 100644
--- a/Documentation/virt/kvm/arm/hypercalls.rst
+++ b/Documentation/virt/kvm/arm/hypercalls.rst
@@ -99,6 +99,16 @@ Revoke access permission from the KVM host to a memory region previously shared
 with ``ARM_SMCCC_KVM_FUNC_MEM_SHARE``. The size of the region is equal to the
 memory protection granule advertised by ``ARM_SMCCC_KVM_FUNC_HYP_MEMINFO``.
 
+Calls to ``ARM_SMCCC_KVM_FUNC_MEM_SHARE`` and ``ARM_SMCCC_KVM_FUNC_MEM_UNSHARE``
+for the same region that overlap in time, for example on different vCPUs, are
+not ordered with respect to each other: a share that has not yet returned may
+take effect after an unshare of the region has returned, leaving the region
+shared. A share of a region that is not yet mapped is restarted after the host
+has mapped it, so such a share can remain pending for as long as the host
+delays the vCPU. A guest must therefore not treat a region as private again
+until every ``ARM_SMCCC_KVM_FUNC_MEM_SHARE`` it issued for the region has
+returned.
+
 +---------------------+-------------------------------------------------------------+
 | Presence:           | Optional; pKVM protected guests only.                       |
 +---------------------+-------------------------------------------------------------+

---
base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
change-id: 20261105-mem-share-unshare-824c26c2cd0b

Best regards,
-- 
Xuheng Li <xuheng@cs.columbia.edu>

[PATCH RFC] KVM: arm64: Document that overlapping MEM_SHARE and MEM_UNSHARE are unordered

[Xuheng Li via B4 Relay]

From: Xuheng Li <xuheng@cs.columbia.edu>

The pKVM guest interface does not say what happens when a protected
guest issues MEM_SHARE and MEM_UNSHARE for the same page concurrently,
for example on two vCPUs.  The two calls are not ordered: a MEM_SHARE
that has not returned yet may take effect after a MEM_UNSHARE of the
same page has returned, and leave the page shared although the vCPU
that unshared it saw SUCCESS.

  vCPU A                         vCPU B
  ------                         ------
  MEM_SHARE(P) issued ...
                                 MEM_SHARE(P)    -> SUCCESS
                                 MEM_UNSHARE(P)  -> SUCCESS
                                 write private data to P
  ... MEM_SHARE(P) takes effect  -> SUCCESS: P is shared again

How long a share can stay pending is up to the host.  When the page is
not yet mapped in the guest's stage-2, pkvm_memshare_call() turns the
call into a data abort for the host (__pkvm_memshare_page_req()),
rewinds the guest's PC and exits; the share is performed from scratch
only when the host runs the vCPU again, with no record of the suspended
call and no lock held across it.  So the host can make vCPU A's share
complete after vCPU B's unshare, and read what B wrote to P.

Document that such calls are unordered, so that a guest knows it must
wait for all its shares of a region to return before treating the
region as private.

Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
We noticed this while studying pKVM's guest memory-sharing interface as
part of our research project, and are not sure whether documenting it is
the right answer.

A guest racing MEM_SHARE and MEM_UNSHARE of the same page on two vCPUs
is arguably asking for trouble, and Linux guests don't do it today.  But
nothing in the interface tells a guest when a page is private again,
and because a share of an unmapped page is suspended until the host
runs the vCPU again, the host controls how long the window stays open.
The alternative to this documentation change would be to record a
suspended MEM_SHARE (for example in the hyp vCPU) so that a later
MEM_UNSHARE of the same page cancels it, or to fail the unshare while a
share of that page is pending.  We'd be happy to work on either if you
prefer a code change.
---
 Documentation/virt/kvm/arm/hypercalls.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/virt/kvm/arm/hypercalls.rst b/Documentation/virt/kvm/arm/hypercalls.rst
index 7400a89aa..ad7082da1 100644
--- a/Documentation/virt/kvm/arm/hypercalls.rst
+++ b/Documentation/virt/kvm/arm/hypercalls.rst
@@ -99,6 +99,16 @@ Revoke access permission from the KVM host to a memory region previously shared
 with ``ARM_SMCCC_KVM_FUNC_MEM_SHARE``. The size of the region is equal to the
 memory protection granule advertised by ``ARM_SMCCC_KVM_FUNC_HYP_MEMINFO``.
 
+Calls to ``ARM_SMCCC_KVM_FUNC_MEM_SHARE`` and ``ARM_SMCCC_KVM_FUNC_MEM_UNSHARE``
+for the same region that overlap in time, for example on different vCPUs, are
+not ordered with respect to each other: a share that has not yet returned may
+take effect after an unshare of the region has returned, leaving the region
+shared. A share of a region that is not yet mapped is restarted after the host
+has mapped it, so such a share can remain pending for as long as the host
+delays the vCPU. A guest must therefore not treat a region as private again
+until every ``ARM_SMCCC_KVM_FUNC_MEM_SHARE`` it issued for the region has
+returned.
+
 +---------------------+-------------------------------------------------------------+
 | Presence:           | Optional; pKVM protected guests only.                       |
 +---------------------+-------------------------------------------------------------+

---
base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
change-id: 20261105-mem-share-unshare-824c26c2cd0b

Best regards,
-- 
Xuheng Li <xuheng@cs.columbia.edu>

Re: [PATCH RFC] KVM: arm64: Document that overlapping MEM_SHARE and MEM_UNSHARE are unordered

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: arm64: hypercall doc gives insufficient condition for private region
--

commit 39c34714b27ef8c9e3ad559a667906cddd707b8b
Author: Xuheng Li <xuheng@cs.columbia.edu>

KVM: arm64: Document that overlapping MEM_SHARE and MEM_UNSHARE are unordered

This commit documents in Documentation/virt/kvm/arm/hypercalls.rst that
overlapping calls to ARM_SMCCC_KVM_FUNC_MEM_SHARE and
ARM_SMCCC_KVM_FUNC_MEM_UNSHARE for the same memory region are unordered with
respect to each other.

> diff --git a/Documentation/virt/kvm/arm/hypercalls.rst b/Documentation/virt/kvm/arm/hypercalls.rst
> index 7400a89aabf88..ad7082da1b9fc 100644
> --- a/Documentation/virt/kvm/arm/hypercalls.rst
> +++ b/Documentation/virt/kvm/arm/hypercalls.rst
> @@ -99,6 +99,16 @@ Revoke access permission from the KVM host to a memory region previously shared
>  with ``ARM_SMCCC_KVM_FUNC_MEM_SHARE``. The size of the region is equal to the
>  memory protection granule advertised by ``ARM_SMCCC_KVM_FUNC_HYP_MEMINFO``.
>  
> +Calls to ``ARM_SMCCC_KVM_FUNC_MEM_SHARE`` and ``ARM_SMCCC_KVM_FUNC_MEM_UNSHARE``
> +for the same region that overlap in time, for example on different vCPUs, are
> +not ordered with respect to each other: a share that has not yet returned may
> +take effect after an unshare of the region has returned, leaving the region
> +shared. A share of a region that is not yet mapped is restarted after the host
> +has mapped it, so such a share can remain pending for as long as the host
> +delays the vCPU. A guest must therefore not treat a region as private again
> +until every ``ARM_SMCCC_KVM_FUNC_MEM_SHARE`` it issued for the region has
> +returned.

[Severity: Medium]
Is waiting until every issued ARM_SMCCC_KVM_FUNC_MEM_SHARE has returned
sufficient for a guest to treat the region as private again?

In the race described by the commit message, if vCPU A issues
ARM_SMCCC_KVM_FUNC_MEM_SHARE while the page is unmapped at stage-2, and vCPU B
issues ARM_SMCCC_KVM_FUNC_MEM_SHARE followed by
ARM_SMCCC_KVM_FUNC_MEM_UNSHARE, vCPU B's __pkvm_guest_unshare_host() returns
the page state to PKVM_PAGE_OWNED in the guest stage-2 and PKVM_NOPAGE in the
host stage-2.

When vCPU A's pending ARM_SMCCC_KVM_FUNC_MEM_SHARE subsequently runs,
arch/arm64/kvm/hyp/nvhe/mem_protect.c:__pkvm_guest_share_host() sees the page
back in PKVM_PAGE_OWNED state and shares it again:

	if (pkvm_getstate(kvm_pgtable_stage2_pte_prot(pte)) != PKVM_PAGE_OWNED)
		goto unlock;
	if (__host_check_page_state_range(phys, PAGE_SIZE, PKVM_NOPAGE))
		goto unlock;

	ret = 0;
	WARN_ON(kvm_pgtable_stage2_map(&vm->pgt, ipa, PAGE_SIZE, phys,
				       pkvm_mkstate(KVM_PGTABLE_PROT_RWX, PKVM_PAGE_SHARED_OWNED),
				       &vcpu->vcpu.arch.stage2_mc, 0));
	WARN_ON(__host_set_page_state_range(phys, PAGE_SIZE, PKVM_PAGE_SHARED_BORROWED));

Once vCPU A's call returns, every ARM_SMCCC_KVM_FUNC_MEM_SHARE issued for the
region has returned, satisfying the documented condition, yet the region is
still shared with the host rather than private.

Should the documentation instead state that every prior
ARM_SMCCC_KVM_FUNC_MEM_SHARE for a region must have returned before issuing
ARM_SMCCC_KVM_FUNC_MEM_UNSHARE for the guest to treat the region as private
once the unshare succeeds?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-mem-share-unshare-v1-1-e4c7d12d5d1d@cs.columbia.edu?part=1