Re: [PATCH v1 1/4] KVM: arm64: Allow early calls to pKVM host_share/unshare_hyp

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] State desynchronization and memory leak when kvm_call_hyp_nvhe(__pkvm_host_share_hyp) fails.
- [High] State desynchronization when kvm_call_hyp_nvhe(__pkvm_host_unshare_hyp) fails.
- [Medium] Memory leak in hyp_trace_buffer_alloc_bpages_backing() when __map_hyp() fails.
--

commit e75dd763b7286e9546dc81b03c493cbab89745cf
Author: Vincent Donnefort <vdonnefort@google.com>

KVM: arm64: Allow early calls to pKVM host_share/unshare_hyp

The hypervisor tracing for pKVM relies on the __pkvm_host_share_hyp and
__pkvm_host_unshare_hyp HVCs. In order to start tracing as early as
possible, allow those two HVCs before the host is deprivileged.

> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index 043495f7fc78b..fb049c40d04f4 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -89,12 +89,12 @@ enum __kvm_host_smccc_func {
>  	__KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs,
>  	__KVM_HOST_SMCCC_FUNC___vgic_v5_save_apr,
>  	__KVM_HOST_SMCCC_FUNC___vgic_v5_restore_vmcr_apr,
> +	__KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp,
> +	__KVM_HOST_SMCCC_FUNC___pkvm_host_unshare_hyp,
>  
>  	MARKER(__KVM_HOST_SMCCC_FUNC_PKVM_ONLY),
>  
>  	/* Hypercalls that are available only when pKVM has finalised. */
> -	__KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp,
> -	__KVM_HOST_SMCCC_FUNC___pkvm_host_unshare_hyp,

[Severity: High]
This isn't a bug introduced by this patch, but while looking at
__pkvm_host_share_hyp, I noticed an issue in share_pfn_hyp() in
arch/arm64/kvm/mmu.c when the hypercall fails:

arch/arm64/kvm/mmu.c:share_pfn_hyp() {
    ...
	rb_insert_color(&this->node, &hyp_shared_pfns);
	ret = kvm_call_hyp_nvhe(__pkvm_host_share_hyp, pfn);
unlock:
	mutex_unlock(&hyp_shared_pfns_lock);

	return ret;
}

If the hypervisor rejects the share operation and returns an error, the
tracking object is left in the RB tree without rolling back the insertion
(using rb_erase and kfree).

Could subsequent calls to share_pfn_hyp() for the same PFN find this stale
node, increment its reference count, and return success without actually
making another hypercall? This seems like it could leave the host thinking
the page is shared while the hypervisor does not.


[Severity: High]
This is a pre-existing issue, but I also noticed a similar problem with
__pkvm_host_unshare_hyp in unshare_pfn_hyp() in arch/arm64/kvm/mmu.c:

arch/arm64/kvm/mmu.c:unshare_pfn_hyp() {
    ...
	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);

	return ret;
}

The tracking node is destroyed unconditionally before verifying that the
hypervisor successfully executed the unshare operation.

If the hypercall fails, does the host irrevocably lose track of the sharing
state? It appears this would create a mismatch where the hypervisor still
considers the page shared, potentially leading to memory corruption or
invalid ownership transitions later.


[Severity: Medium]
This is a pre-existing issue, but since tracing relies on these hypercalls,
I spotted a memory leak in the error path of hyp_trace_load() in
arch/arm64/kvm/hyp_trace.c:

arch/arm64/kvm/hyp_trace.c:hyp_trace_load() {
    ...
	ret = hyp_trace_buffer_alloc_bpages_backing(trace_buffer, size);
	if (ret)
		goto err_free_desc;
    ...
err_free_backing:
	hyp_trace_buffer_free_bpages_backing(trace_buffer);

err_free_desc:
	free_pages_exact(desc, desc_size);
	trace_buffer->desc = NULL;

	return ERR_PTR(ret);
}

If __map_hyp() fails inside hyp_trace_buffer_alloc_bpages_backing(), it
returns the error to hyp_trace_load(), which then jumps directly to
err_free_desc.

Will this bypass the err_free_backing label and cause the backing pages
allocated in hyp_trace_buffer_alloc_bpages_backing() to be permanently
leaked?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612142245.1015744-1-vdonnefort@google.com?part=1