linux-kernel - Re: [PATCH] KVM: arm/arm64: inject an async abort to host if SEA handling failed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 24 Apr 2019 08:46:39 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Dongjiu Geng <gengdongjiu@...wei.com>, christoffer.dall@....com,
        james.morse@....com, julien.thierry@....com,
        suzuki.poulose@....com, linux-arm-kernel@...ts.infradead.org,
        kvmarm@...ts.cs.columbia.edu, linux-kernel@...r.kernel.org,
        zhengxiang9@...wei.com
Subject: Re: [PATCH] KVM: arm/arm64: inject an async abort to host if SEA
 handling failed

On 24/04/2019 05:41, Dongjiu Geng wrote:
> If host failed to handle the SEA, KVM should inject an async abort
> to guest for both SEA data and instruction abort, but it currently
> only handles the data abort, so correct it.
> 
> Cc: James Morse <james.morse@....com>
> Cc: Xiang Zheng <zhengxiang9@...wei.com>
> Signed-off-by: Dongjiu Geng <gengdongjiu@...wei.com>
> ---
>  virt/kvm/arm/mmu.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index 27c9583..5882516 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -1924,14 +1924,13 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  		/*
>  		 * For RAS the host kernel may handle this abort.
>  		 * There is no need to pass the error into the guest.
> +		 * If host failed to handle it, inject an async abort
> +		 * to guest.
>  		 */
> -		if (!kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_hsr(vcpu)))
> -			return 1;
> -
> -		if (unlikely(!is_iabt)) {
> +		if (kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_hsr(vcpu)))

We've so far excluded instruction aborts from the delivery of a virtual
SError. You now decide to inject an SError in all cases, but your commit
message doesn't explain *why* this is a sensible thing to do.

I'm not saying that this patch is wrong (the IABT handling predates RAS
by several years and was  designed to deal with with badly behaved guest
rather than flaky HW), but I'd like to understand why you think it is right.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...