Re: [PATCH v5 09/15] KVM: arm64: Handle HDBSS faults

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] arm64: KVM checks DABT-specific HDBSSF bit on Instruction Aborts
- [High] KVM: guest-reachable WARN_ON_ONCE assertions in HDBSS fault handler
--

commit 93c9cd523cd96ee876cb40ad4d3ecad32cc8aeff
Author: Eillon <yezhenyu2@huawei.com>

KVM: arm64: Handle HDBSS faults

This commit introduces handling for hardware stopped promoting writable-clean
descriptors, which raises a stage-2 Permission fault with ESR_EL2.ISS2.HDBSSF.
It dispatches these from kvm_handle_guest_abort() via a new helper to clear
the fault and report -EFAULT.

> diff --git a/arch/arm64/kvm/dirty_bit.c b/arch/arm64/kvm/dirty_bit.c
> index be0d12555c848..893a8c4248bcc 100644
> --- a/arch/arm64/kvm/dirty_bit.c
> +++ b/arch/arm64/kvm/dirty_bit.c
[ ... ]
> +int kvm_handle_hdbss_fault(struct kvm_vcpu *vcpu)
> +{
> +	u64 prod;
> +	u64 fsc;
> +
> +	if (WARN_ON_ONCE(!system_supports_hdbss()))
> +		return -EFAULT;
> +
> +	if (WARN_ON_ONCE(!vcpu_hdbss_enabled(vcpu)))
> +		return -EFAULT;

[Severity: High]
Are these WARN_ON_ONCE assertions safe to expose on a guest-reachable path?

If a guest manages to inadvertently route execution here (for instance, via 
an Instruction Abort matching the HDBSSF bit in its ISS2 field), these 
assertions can be triggered directly by the guest.

If the host has panic_on_warn enabled, this could lead to a host kernel
panic and a complete denial of service. Should these be standard error
returns without the warnings instead?

> +
> +	prod = read_sysreg_s(SYS_HDBSSPROD_EL2);
> +	fsc = FIELD_GET(HDBSSPROD_EL2_FSC_MASK, prod);
> +
> +	if (fsc == HDBSSPROD_EL2_FSC_OK)
> +		/* Buffer full: the exit path drained it before handle_exit. */
> +		return 1;
> +
> +	if (fsc != HDBSSPROD_EL2_FSC_ExternalAbort &&
> +	    fsc != HDBSSPROD_EL2_FSC_GPF)
> +		WARN_ONCE(1,

[Severity: High]
Similarly, could this WARN_ONCE be triggered by a guest-driven event?

> +			  "Unexpected HDBSS fault type, FSC: 0x%llx (prod=0x%llx, vcpu=%d)\n",
> +			  fsc, prod, vcpu->vcpu_id);
> +
> +	/* Clear FSC so hardware dirty state updates can resume. */
> +	write_sysreg_s(prod & ~HDBSSPROD_EL2_FSC_MASK, SYS_HDBSSPROD_EL2);
> +	return -EFAULT;
> +}
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 85a98d2c23a99..7bf82d65041c8 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
[ ... ]
> @@ -2315,6 +2316,9 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
>  
>  	is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
>  
> +	if (esr_iss2_is_hdbssf(esr))
> +		return kvm_handle_hdbss_fault(vcpu);
> +

[Severity: High]
Does this inadvertently check the HDBSSF bit for Instruction Aborts?

The esr_iss2_is_hdbssf() helper in arch/arm64/include/asm/esr.h extracts
the ISS2 bit without verifying the Exception Class. Because the ISS layout
is per-EC, an Instruction Abort's ISS2 field might coincidentally have this
bit set for an entirely unrelated reason. 

If that happens, a valid Instruction Abort would be misidentified as a
HDBSS fault, routing execution into kvm_handle_hdbss_fault() and causing
guest breakage. Should this check be gated by !is_iabt or a specific
Exception Class check?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929103655.85107-1-zhengtian10@huawei.com?part=9