Re: [PATCH v11 0/8] KVM: allow mapping non-refcounted pages

[Sean Christopherson]

On Thu, Mar 14, 2024, Christian König wrote:
> Am 14.03.24 um 12:31 schrieb David Stevens:
> > On Thu, Mar 14, 2024 at 6:20 PM Christian König <christiankoenig@xxxxxxx> wrote:
> > > > > > > > Well as far as I can see Christoph rejects the complexity coming with the
> > > > > > > > approach of sometimes grabbing the reference and sometimes not.
> > > > > > > Unless I've wildly misread multiple threads, that is not Christoph's objection.
> > > > > > >    From v9 (https://lore.kernel.org/all/ZRpiXsm7X6BFAU%2Fy@xxxxxxxxxxxxx):
> > > > > > > 
> > > > > > >      On Sun, Oct 1, 2023 at 11:25 PM Christoph Hellwig<hch@xxxxxxxxxxxxx>   wrote:
> > > > > > >      >
> > > > > > >      > On Fri, Sep 29, 2023 at 09:06:34AM -0700, Sean Christopherson wrote:
> > > > > > >      > > KVM needs to be aware of non-refcounted struct page memory no matter what; see
> > > > > > >      > > CVE-2021-22543 and, commit f8be156be163 ("KVM: do not allow mapping valid but
> > > > > > >      > > non-reference-counted pages").  I don't think it makes any sense whatsoever to
> > > > > > >      > > remove that code and assume every driver in existence will do the right thing.
> > > > > > >      >
> > > > > > >      > Agreed.
> > > > > > >      >
> > > > > > >      > >
> > > > > > >      > > With the cleanups done, playing nice with non-refcounted paged instead of outright
> > > > > > >      > > rejecting them is a wash in terms of lines of code, complexity, and ongoing
> > > > > > >      > > maintenance cost.
> > > > > > >      >
> > > > > > >      > I tend to strongly disagree with that, though.  We can't just let these
> > > > > > >      > non-refcounted pages spread everywhere and instead need to fix their
> > > > > > >      > usage.
> > > > > > And I can only repeat myself that I completely agree with Christoph here.
> > > > > I am so confused.  If you agree with Christoph, why not fix the TTM allocations?
> > > > Because the TTM allocation isn't broken in any way.
> > > > 
> > > > See in some configurations TTM even uses the DMA API for those
> > > > allocations and that is actually something Christoph coded.
> > > > 
> > > > What Christoph is really pointing out is that absolutely nobody should
> > > > put non-refcounted pages into a VMA, but again this isn't something
> > > > TTM does. What TTM does instead is to work with the PFN and puts that
> > > > into a VMA.
> > > > 
> > > > It's just that then KVM comes along and converts the PFN back into a
> > > > struct page again and that is essentially what causes all the
> > > > problems, including CVE-2021-22543.
> > Does Christoph's objection come from my poorly worded cover letter and
> > commit messages, then?
> 
> Yes, that could certainly be.
> 
> > Fundamentally, what this series is doing is
> > allowing pfns returned by follow_pte to be mapped into KVM's shadow
> > MMU without inadvertently translating them into struct pages.
> 
> As far as I can tell that is really the right thing to do. Yes.

Christoph,

Can you please confirm that you don't object to KVM using follow_pte() to get
PFNs which happen to have an associated struct page?  We've gone in enough circles...