linux-kernel - Re: [PATCH 2/2] KVM: x86/mmu: Recover NX Huge pages belonging to TDP MMU under MMU read lock

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: <CAHVum0dMmjd0b68aYJh8=F8kYShM3bQh0eXDL1+m27Om5YaDCg@mail.gmail.com>
Date: Fri, 23 Aug 2024 14:41:44 -0700
From: Vipin Sharma <vipinsh@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: pbonzini@...hat.com, dmatlack@...gle.com, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] KVM: x86/mmu: Recover NX Huge pages belonging to TDP
 MMU under MMU read lock

On Fri, Aug 23, 2024 at 1:45 PM Sean Christopherson <seanjc@...gle.com> wrote:
>
> On Fri, Aug 23, 2024, Vipin Sharma wrote:
> > and if that didn't suceed then return the error and let caller handle however
> > they want to.  NX huge page recovery should be tolerant of this zapping
> > failure and move on to the next shadow page.
>
> Again, I agree, but that is orthogonal to the WARN I am suggesting.  The intent
> of the WARN isn't to detect that zapping failed, it's to flag that the impossible
> has happened.
>
> > May be we can put WARN if NX huge page recovery couldn't zap any pages during
> > its run time. For example, if it was supposed to zap 10 pages and it couldn't
> > zap any of them then print using WARN_ON_ONCE.
>
> Why?  It's improbable, but absolutely possible that zapping 10 SPTEs could fail.
> Would it make sense to publish a stat so that userspace can alert on NX huge page
> recovery not being as effective as it should be?  Maybe.  But the kernel should
> never WARN because an unlikely scenario occurred.

I misunderstood your WARN requirement in the response to this patch
and that's why suggested that if you are preferring WARN this much :)
maybe we can move it out outside and print only if nothing got zapped.
Glad it's not.

We don't need stat,  userspace can use page stats and NX parameters to
observe if it is working effectively or not.

> If you look at all the KVM_MMU_WARN_ON() and (hopefully) WARN_ON_ONCE() calls in
> KVM x86, they're either for things that should never happen absent software or
> hardware bugs, or to ensure future changes update all relevant code paths.
>
> The WARN I am suggesting is the same.  It can only fire if there's a hardware
> bug, a KVM bug, or if KVM changes how it behaves, e.g. starts doing A/D tracking
> on non-leaf SPTEs.
>

I responded to this email before moving on to your next email :)

I agree with WARN there which is for checking if new SPTE is pointing
to the old page table. I have a few other comments (not related to
WARN), I will respond there.

Thanks!