linux-kernel - Re: [PATCH] KVM: arm64: nv: Allow shadow stage 2 read fault

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]

Message-ID: <l4ge3jp2leuzxkpn7nowptl7hjzeiq6amqowibycdhuahgafnw@o3dcvj7njix5>
Date: Tue, 26 Aug 2025 21:55:11 +0800
From: Wei-Lin Chang <r09922117@...e.ntu.edu.tw>
To: Oliver Upton <oliver.upton@...ux.dev>
Cc: linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev, 
	linux-kernel@...r.kernel.org, Marc Zyngier <maz@...nel.org>, Joey Gouly <joey.gouly@....com>, 
	Suzuki K Poulose <suzuki.poulose@....com>, Zenghui Yu <yuzenghui@...wei.com>, 
	Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>
Subject: Re: [PATCH] KVM: arm64: nv: Allow shadow stage 2 read fault

Hi Oliver,

On Fri, Aug 22, 2025 at 02:25:43AM -0700, Oliver Upton wrote:
> Hi Wei-Lin,
> 
> You've been finding some good stuff with nested, thank you :)
> 
> On Fri, Aug 22, 2025 at 11:18:53AM +0800, Wei-Lin Chang wrote:
> > I am able to trigger this error with a modified L1 KVM, but I do realize
> > this requires L1 to be very strange (or even just wrong) so I understand
> > if we don't want to handle this kind of edge case. On the other hand,
> > could there also be other ways to trigger this that I have not thought
> > of?
> 
> The architecture is pretty unambiguous here that the stage-2 can
> represent a translation w/o read permission.
> 
> > Another thing is that this change lets L1 get away with not flushing the
> > TLB, but TLBs are ephemeral so it's fine in this aspect, however I'm not
> > sure if there are other considerations.
> 
> FEAT_ETS3 is an interesting one since it provides software with ordering
> expectations around MMU faults (including permission faults) and updates
> to the translation tables. For KVM's shadow stage-2 to comply we need to
> re-walk the guest's stage-2 before injecting the fault at L1.

Thanks for telling me this, I haven't noticed ETS before, let me learn
about it.

> 
> > ---
> >  arch/arm64/kvm/mmu.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 1c78864767c5c..41017ca579b19 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -1508,8 +1508,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> >  	exec_fault = kvm_vcpu_trap_is_exec_fault(vcpu);
> >  	VM_BUG_ON(write_fault && exec_fault);
> >  
> > -	if (fault_is_perm && !write_fault && !exec_fault) {
> > -		kvm_err("Unexpected L2 read permission error\n");
> > +	if (fault_is_perm && !write_fault && !exec_fault && !nested) {
> > +		kvm_err("Unexpected S2 read permission error\n");
> >  		return -EFAULT;
> >  	}
> 
> Hmm... I'm also willing to just delete this check altogether. The
> likelihood of KVM creating a stage-2 PTE w/o read permission is rather
> low.

Agreed, and this is in agreement with Marc, let me drop this in the next
version.

Thanks,
Wei-Lin Chang

> 
> Thanks,
> Oliver