linux-kernel - Re: [UNVERIFIED SENDER] 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]

Date:   Fri, 6 Sep 2019 15:02:07 +0200
From:   Alexander Graf <graf@...zon.com>
To:     Marc Zyngier <maz@...nel.org>,
        Christoffer Dall <christoffer.dall@....com>
CC:     Daniel P. Berrangé <berrange@...hat.com>,
        "Heinrich Schuchardt" <xypron.glpk@....de>,
        lkml - Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Stefan Hajnoczi <stefanha@...hat.com>,
        <kvmarm@...ts.cs.columbia.edu>,
        arm-mail-list <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [UNVERIFIED SENDER] Re: [PATCH 1/1] KVM: inject data abort if
 instruction cannot be decoded



On 06.09.19 14:34, Marc Zyngier wrote:
> On 06/09/2019 13:08, Alexander Graf wrote:
>>
>>
>> On 06.09.19 10:00, Christoffer Dall wrote:
>>> On Thu, Sep 05, 2019 at 02:09:18PM +0100, Marc Zyngier wrote:
> 
> [...]
> 
>>>>> @@ -673,6 +694,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>>>>    		ret = kvm_handle_mmio_return(vcpu, vcpu->run);
>>>>>    		if (ret)
>>>>>    			return ret;
>>>>> +	} else if (run->exit_reason == KVM_EXIT_ARM_NISV) {
>>>>> +		kvm_inject_undefined(vcpu);
>>>>
>>>> Just to make sure I understand: Is the expectation here that userspace
>>>> could clear the exit reason if it managed to handle the exit? And
>>>> otherwise we'd inject an UNDEF on reentry?
>>>>
>>>
>>> Yes, but I think we should change that to an external abort.  I'll test
>>> something and send a proper patch with more clear documentation.
>>
>> Why not leave the injection to user space in any case? API wise there is
>> no need to be backwards compatible, as we require the CAP to be enabled,
>> right?
>>
>> IMHO it should be 100% a policy decision in user space whether to
>> emulate and what type of exception to inject, if anything.
> 
> The exception has to be something that the trapped instruction can
> actually generate. An UNDEF is definitely wrong, as the guest would have
> otherwise UNDEF'd at EL1, and KVM would have never seen it. You cannot
> deviate from the rule of architecture, and userspace feels like the
> wrong place to enforce it.

There are multiple viable options user space has:

   1) Trigger an external abort
   2) Emulate the instruction in user space
   3) Inject a PV mechanism into the guest to emulate the insn inside 
guest space

Why should we treat 1) any different from 2) or 3)? Why is there a "fast 
path" for the external abort, on an exit that is not performance 
critical or has any other reason to get special attention from kernel 
space. All we're doing is add more code in a privileged layer for a case 
that realistically should never occur in the first place.


Alex



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879