[PATCH v2 0/3] KVM: arm64: Fix host/hyp tracking on share/unshare hypercall failure

[PATCH v2 0/3] KVM: arm64: Fix host/hyp tracking on share/unshare hypercall failure

[tabba]

Hi folks,

The first two started as bugs I found testing Sashiko locally with
fixes to review-prompts. The third grew out of the v1 discussion.

share_pfn_hyp() and unshare_pfn_hyp() in arch/arm64/kvm/mmu.c maintain
a host-side RB-tree mirroring the set of pages shared with EL2. The
hypercalls they wrap can fail (page-state mismatch, EL2 refcount still
held), and neither the per-pfn helpers nor the multi-page wrappers
cleaned up correctly on failure:

- share_pfn_hyp() left its tracking node in the tree on failure,
  leaking the allocation and presenting a phantom share to a later
  unshare (patch 1).

- unshare_pfn_hyp() erased its tracking node before the hypercall, so
  on failure the host lost its record while EL2 still owned the share
  (patch 2).

- kvm_share_hyp() returned on the first per-page failure, stranding the
  pages already shared by that call: the caller treats the whole range
  as failed and never unshares them (patch 3).

As Vincent and Marc noted on v1, none of this compromises isolation. A
page that cannot be unshared is simply leaked: it stays shared with the
hypervisor and is no longer reusable for pKVM. So kvm_share_hyp() now
rolls back on failure, and the unshare WARN_ON()s are left non-fatal
and documented rather than promoted to BUG_ON(). The system keeps
running, and only later pKVM reuse of a leaked page would fail. We do
not expect any of these paths to trigger in practice.

Severity is low and this can wait for 7.2. Patch 3 builds on patch 2,
otherwise they are independent.

Changes since v1:
 - New patch 3: roll back partial shares in kvm_share_hyp(); document
   the deliberate leak-on-WARN in kvm_unshare_hyp() (Vincent, Marc).
 - Patches 1 and 2 functionally unchanged (patch 2 gains the call-site
   comment).
 - v1: https://lore.kernel.org/all/20260529074341.2271950-1-tabba@google.com/

Cheers,
/fuad

Fuad Tabba (3):
  KVM: arm64: Free hyp-share tracking node when share hypercall fails
  KVM: arm64: Avoid host/hyp share desync on unshare hypercall failure
  KVM: arm64: Roll back partial shares on kvm_share_hyp() failure

 arch/arm64/kvm/mmu.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

-- 
2.54.0.929.g9b7fa37559-goog

[PATCH v2 1/3] KVM: arm64: Free hyp-share tracking node when share hypercall fails

[tabba]

share_pfn_hyp() inserts a tracking node into hyp_shared_pfns and
then invokes __pkvm_host_share_hyp. If the hypercall rejects the
share (page-state mismatch at EL2), the node stays in the tree
with refcount 1: a phantom share that leaks the allocation and
that a later unshare will trust.

Erase the node and free it on hypercall failure.

Fixes: a83e2191b7f1 ("KVM: arm64: pkvm: Refcount the pages shared with EL2")
Reported-by: Sashiko (local):gemini-3.1-pro
Suggested-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/mmu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 4da9281312eb..4a928fb003ff 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -501,6 +501,10 @@ static int share_pfn_hyp(u64 pfn)
 	rb_link_node(&this->node, parent, node);
 	rb_insert_color(&this->node, &hyp_shared_pfns);
 	ret = kvm_call_hyp_nvhe(__pkvm_host_share_hyp, pfn);
+	if (ret) {
+		rb_erase(&this->node, &hyp_shared_pfns);
+		kfree(this);
+	}
 unlock:
 	mutex_unlock(&hyp_shared_pfns_lock);
 
-- 
2.54.0.929.g9b7fa37559-goog

[PATCH v2 2/3] KVM: arm64: Avoid host/hyp share desync on unshare hypercall failure

[tabba]

unshare_pfn_hyp() erases the tracking node from hyp_shared_pfns
and frees it before invoking __pkvm_host_unshare_hyp. If the
hypercall fails (e.g. EL2 refcount still held, or page-state
mismatch), the host loses its record while EL2 still holds the
share, breaking later share/unshare attempts on the same pfn.

Invoke the hypercall first; erase and free only on success.

Document at the kvm_unshare_hyp() call site that the WARN_ON() is
left non-fatal: a failed unshare leaks the page (it stays shared
with the hypervisor) but breaks no isolation guarantee.

Fixes: 52b28657ebd7 ("KVM: arm64: pkvm: Unshare guest structs during teardown")
Reported-by: Sashiko (local):gemini-3.1-pro
Suggested-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/mmu.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 4a928fb003ff..e08503e89fc4 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -524,13 +524,17 @@ static int unshare_pfn_hyp(u64 pfn)
 		goto unlock;
 	}
 
-	this->count--;
-	if (this->count)
+	if (this->count > 1) {
+		this->count--;
+		goto unlock;
+	}
+
+	ret = kvm_call_hyp_nvhe(__pkvm_host_unshare_hyp, pfn);
+	if (ret)
 		goto unlock;
 
 	rb_erase(&this->node, &hyp_shared_pfns);
 	kfree(this);
-	ret = kvm_call_hyp_nvhe(__pkvm_host_unshare_hyp, pfn);
 unlock:
 	mutex_unlock(&hyp_shared_pfns_lock);
 
@@ -581,6 +585,11 @@ void kvm_unshare_hyp(void *from, void *to)
 	end = PAGE_ALIGN(__pa(to));
 	for (cur = start; cur < end; cur += PAGE_SIZE) {
 		pfn = __phys_to_pfn(cur);
+		/*
+		 * A failed unshare leaks the page: it stays shared with the
+		 * hypervisor and is no longer reusable for pKVM. No isolation
+		 * guarantee is broken, and this is not expected in practice.
+		 */
 		WARN_ON(unshare_pfn_hyp(pfn));
 	}
 }
-- 
2.54.0.929.g9b7fa37559-goog

[PATCH v2 3/3] KVM: arm64: Roll back partial shares on kvm_share_hyp() failure

[tabba]

kvm_share_hyp() shares a range one page at a time. If share_pfn_hyp()
fails partway through, the pages already shared by this call are left
shared, while the caller treats the whole range as failed and never
unshares them.

Unshare those pages before returning the error. If an unshare itself
fails the page is leaked: it stays shared with the hypervisor and is
no longer reusable for pKVM, but no isolation guarantee is broken, so
WARN and continue. Not expected in practice.

Fixes: a83e2191b7f1 ("KVM: arm64: pkvm: Refcount the pages shared with EL2")
Suggested-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/mmu.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index e08503e89fc4..8811ad60cf72 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -544,8 +544,8 @@ static int unshare_pfn_hyp(u64 pfn)
 int kvm_share_hyp(void *from, void *to)
 {
 	phys_addr_t start, end, cur;
+	int ret = 0;
 	u64 pfn;
-	int ret;
 
 	if (is_kernel_in_hyp_mode())
 		return 0;
@@ -567,10 +567,24 @@ int kvm_share_hyp(void *from, void *to)
 		pfn = __phys_to_pfn(cur);
 		ret = share_pfn_hyp(pfn);
 		if (ret)
-			return ret;
+			break;
 	}
 
-	return 0;
+	if (!ret)
+		return 0;
+
+	/*
+	 * Roll back the pages shared by this call. A failed unshare leaks
+	 * the page (it stays shared with the hypervisor and is no longer
+	 * reusable for pKVM) but breaks no isolation guarantee, so warn and
+	 * continue. Not expected in practice.
+	 */
+	for (end = cur, cur = start; cur < end; cur += PAGE_SIZE) {
+		pfn = __phys_to_pfn(cur);
+		WARN_ON(unshare_pfn_hyp(pfn));
+	}
+
+	return ret;
 }
 
 void kvm_unshare_hyp(void *from, void *to)
-- 
2.54.0.929.g9b7fa37559-goog

Re: [PATCH v2 0/3] KVM: arm64: Fix host/hyp tracking on share/unshare hypercall failure

[Vincent Donnefort]

On Fri, May 29, 2026 at 01:17:52PM +0100, tabba@google.com wrote:
> Hi folks,
> 
> The first two started as bugs I found testing Sashiko locally with
> fixes to review-prompts. The third grew out of the v1 discussion.
> 
> share_pfn_hyp() and unshare_pfn_hyp() in arch/arm64/kvm/mmu.c maintain
> a host-side RB-tree mirroring the set of pages shared with EL2. The
> hypercalls they wrap can fail (page-state mismatch, EL2 refcount still
> held), and neither the per-pfn helpers nor the multi-page wrappers
> cleaned up correctly on failure:
> 
> - share_pfn_hyp() left its tracking node in the tree on failure,
>   leaking the allocation and presenting a phantom share to a later
>   unshare (patch 1).
> 
> - unshare_pfn_hyp() erased its tracking node before the hypercall, so
>   on failure the host lost its record while EL2 still owned the share
>   (patch 2).
> 
> - kvm_share_hyp() returned on the first per-page failure, stranding the
>   pages already shared by that call: the caller treats the whole range
>   as failed and never unshares them (patch 3).
> 
> As Vincent and Marc noted on v1, none of this compromises isolation. A
> page that cannot be unshared is simply leaked: it stays shared with the
> hypervisor and is no longer reusable for pKVM. So kvm_share_hyp() now
> rolls back on failure, and the unshare WARN_ON()s are left non-fatal
> and documented rather than promoted to BUG_ON(). The system keeps
> running, and only later pKVM reuse of a leaked page would fail. We do
> not expect any of these paths to trigger in practice.
> 
> Severity is low and this can wait for 7.2. Patch 3 builds on patch 2,
> otherwise they are independent.
> 
> Changes since v1:
>  - New patch 3: roll back partial shares in kvm_share_hyp(); document
>    the deliberate leak-on-WARN in kvm_unshare_hyp() (Vincent, Marc).
>  - Patches 1 and 2 functionally unchanged (patch 2 gains the call-site
>    comment).
>  - v1: https://lore.kernel.org/all/20260529074341.2271950-1-tabba@google.com/
> 
> Cheers,
> /fuad

For the whole series:

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>

> 
> Fuad Tabba (3):
>   KVM: arm64: Free hyp-share tracking node when share hypercall fails
>   KVM: arm64: Avoid host/hyp share desync on unshare hypercall failure
>   KVM: arm64: Roll back partial shares on kvm_share_hyp() failure
> 
>  arch/arm64/kvm/mmu.c | 39 +++++++++++++++++++++++++++++++++------
>  1 file changed, 33 insertions(+), 6 deletions(-)
> 
> -- 
> 2.54.0.929.g9b7fa37559-goog
>