Re: [PATCH stable-5.8] KVM: arm64: Assume write fault on S1PTW permission fault on instruction fetch

[Marc Zyngier <maz@kernel.org>]

On 2020-09-29 08:01, Greg KH wrote:
> On Tue, Sep 29, 2020 at 01:16:34AM +0530, Naresh Kamboju wrote:
>> On Mon, 28 Sep 2020 at 23:16, Greg KH <gregkh@linuxfoundation.org> 
>> wrote:
>> >
>> > On Mon, Sep 28, 2020 at 06:18:50PM +0100, Marc Zyngier wrote:
>> > > Commit c4ad98e4b72cb5be30ea282fce935248f2300e62 upstream.
>> > >
>> > > KVM currently assumes that an instruction abort can never be a write.
>> > > This is in general true, except when the abort is triggered by
>> > > a S1PTW on instruction fetch that tries to update the S1 page tables
>> > > (to set AF, for example).
>> > >
>> > > This can happen if the page tables have been paged out and brought
>> > > back in without seeing a direct write to them (they are thus marked
>> > > read only), and the fault handling code will make the PT executable(!)
>> > > instead of writable. The guest gets stuck forever.
>> > >
>> > > In these conditions, the permission fault must be considered as
>> > > a write so that the Stage-1 update can take place. This is essentially
>> > > the I-side equivalent of the problem fixed by 60e21a0ef54c ("arm64: KVM:
>> > > Take S1 walks into account when determining S2 write faults").
>> > >
>> > > Update kvm_is_write_fault() to return true on IABT+S1PTW, and introduce
>> > > kvm_vcpu_trap_is_exec_fault() that only return true when no faulting
>> > > on a S1 fault. Additionally, kvm_vcpu_dabt_iss1tw() is renamed to
>> > > kvm_vcpu_abt_iss1tw(), as the above makes it plain that it isn't
>> > > specific to data abort.
>> > >
>> > > Signed-off-by: Marc Zyngier <maz@kernel.org>
>> > > Reviewed-by: Will Deacon <will@kernel.org>
>> > > Cc: stable@vger.kernel.org
>> > > Link: https://lore.kernel.org/r/20200915104218.1284701-2-maz@kernel.org
>> >
>> > Thanks for all 3 of these, now queued up!
>> 
>> stable rc branch 4.19 arm64 build broken.
>> 
>> ../arch/arm64/kvm/../../../virt/kvm/arm/mmu.c:1283:13: error:
>> redefinition of ‘kvm_is_write_fault’
>>  1283 | static bool kvm_is_write_fault(struct kvm_vcpu *vcpu)
>>       |             ^~~~~~~~~~~~~~~~~~
>> 
>> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> 
> Thanks, I'll go drop this patch from the 4.19.y queue and wait for a
> fixed up version from Marc.

Right. I have no idea what I tested yesterday, but clearly this didn't
stand a chance to even compile on arm64... :-( Funnily enough, 32bit ARM
(which nobody cares about when it comes to KVM) was just fine. Bah.

Apologies for the noise, v2 coming once I have had my second coffee...

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