linux-kernel - Re: [PATCH 1/1] KVM: inject data abort if instruction cannot be decoded

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190905092039.GG32415@stefanha-x1.localdomain>
Date:   Thu, 5 Sep 2019 10:20:39 +0100
From:   Stefan Hajnoczi <stefanha@...hat.com>
To:     Heinrich Schuchardt <xypron.glpk@....de>
Cc:     Marc Zyngier <marc.zyngier@....com>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry@....com>,
        Suzuki K Pouloze <suzuki.poulose@....com>,
        Peter Maydell <peter.maydell@...aro.org>,
        Daniel P . Berrangé <berrange@...hat.com>,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] KVM: inject data abort if instruction cannot be
 decoded

On Wed, Sep 04, 2019 at 08:07:36PM +0200, Heinrich Schuchardt wrote:
> If an application tries to access memory that is not mapped, an error
> ENOSYS, "load/store instruction decoding not implemented" may occur.
> QEMU will hang with a register dump.
> 
> Instead create a data abort that can be handled gracefully by the
> application running in the virtual environment.
> 
> Now the virtual machine can react to the event in the most appropriate
> way - by recovering, by writing an informative log, or by rebooting.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@....de>
> ---
>  virt/kvm/arm/mmio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/arm/mmio.c b/virt/kvm/arm/mmio.c
> index a8a6a0c883f1..0cbed7d6a0f4 100644
> --- a/virt/kvm/arm/mmio.c
> +++ b/virt/kvm/arm/mmio.c
> @@ -161,8 +161,8 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  		if (ret)
>  			return ret;
>  	} else {
> -		kvm_err("load/store instruction decoding not implemented\n");
> -		return -ENOSYS;
> +		kvm_inject_dabt(vcpu, kvm_vcpu_get_hfar(vcpu));
> +		return 1;

I see this more as a temporary debugging hack than something to merge.

It sounds like in your case the guest environment provided good
debugging information and you preferred it over debugging this from the
host side.  That's fine, but allowing the guest to continue running in
the general case makes it much harder to track down the root cause of a
problem because many guest CPU instructions may be executed after the
original problem occurs.  Other guest software may fail silently in
weird ways.  IMO it's best to fail early.

Stefan

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)