Received-SPF: pass (google.com: domain of linux-kernel+bounces-103430-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 14 Mar 2024 07:45:42 -0700
In-Reply-To: <985fd7f8-f8dd-4ce4-aa07-7e47728e3ebd@amd.com>
Precedence: bulk
Mime-Version: 1.0
References: <0b109bc4-ee4c-4f13-996f-b89fbee09c0b@amd.com> <ZfG801lYHRxlhZGT@google.com>
 <9e604f99-5b63-44d7-8476-00859dae1dc4@amd.com> <ZfHKoxVMcBAMqcSC@google.com>
 <93df19f9-6dab-41fc-bbcd-b108e52ff50b@amd.com> <ZfHhqzKVZeOxXMnx@google.com>
 <c84fcf0a-f944-4908-b7f6-a1b66a66a6bc@amd.com> <d2a95b5c-4c93-47b1-bb5b-ef71370be287@amd.com>
 <CAD=HUj5k+N+zrv-Yybj6K3EvfYpfGNf-Ab+ov5Jv+Zopf-LJ+g@mail.gmail.com> <985fd7f8-f8dd-4ce4-aa07-7e47728e3ebd@amd.com>
Message-ID: <ZfMNGMopN_Ncy0mf@google.com>
Subject: Re: [PATCH v11 0/8] KVM: allow mapping non-refcounted pages
From: Sean Christopherson <seanjc@google.com>
To: "Christian =?utf-8?B?S8O2bmln?=" <christian.koenig@amd.com>
Cc: David Stevens <stevensd@chromium.org>, Christoph Hellwig <hch@infradead.org>, 
	Paolo Bonzini <pbonzini@redhat.com>, Yu Zhang <yu.c.zhang@linux.intel.com>, 
	Isaku Yamahata <isaku.yamahata@gmail.com>, Zhi Wang <zhi.wang.linux@gmail.com>, 
	Maxim Levitsky <mlevitsk@redhat.com>, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, 
	kvm@vger.kernel.org
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Mar 14, 2024, Christian K=C3=B6nig wrote:
> Am 14.03.24 um 12:31 schrieb David Stevens:
> > On Thu, Mar 14, 2024 at 6:20=E2=80=AFPM Christian K=C3=B6nig <christian=
koenig@amd.com> wrote:
> > > > > > > > Well as far as I can see Christoph rejects the complexity c=
oming with the
> > > > > > > > approach of sometimes grabbing the reference and sometimes =
not.
> > > > > > > Unless I've wildly misread multiple threads, that is not Chri=
stoph's objection.
> > > > > > >    From v9 (https://lore.kernel.org/all/ZRpiXsm7X6BFAU%2Fy@in=
fradead.org):
> > > > > > >=20
> > > > > > >      On Sun, Oct 1, 2023 at 11:25=E2=80=AFPM Christoph Hellwi=
g<hch@infradead.org>   wrote:
> > > > > > >      >
> > > > > > >      > On Fri, Sep 29, 2023 at 09:06:34AM -0700, Sean Christo=
pherson wrote:
> > > > > > >      > > KVM needs to be aware of non-refcounted struct page =
memory no matter what; see
> > > > > > >      > > CVE-2021-22543 and, commit f8be156be163 ("KVM: do no=
t allow mapping valid but
> > > > > > >      > > non-reference-counted pages").  I don't think it mak=
es any sense whatsoever to
> > > > > > >      > > remove that code and assume every driver in existenc=
e will do the right thing.
> > > > > > >      >
> > > > > > >      > Agreed.
> > > > > > >      >
> > > > > > >      > >
> > > > > > >      > > With the cleanups done, playing nice with non-refcou=
nted 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 nee=
d to fix their
> > > > > > >      > usage.
> > > > > > And I can only repeat myself that I completely agree with Chris=
toph here.
> > > > > I am so confused.  If you agree with Christoph, why not fix the T=
TM allocations?
> > > > Because the TTM allocation isn't broken in any way.
> > > >=20
> > > > See in some configurations TTM even uses the DMA API for those
> > > > allocations and that is actually something Christoph coded.
> > > >=20
> > > > What Christoph is really pointing out is that absolutely nobody sho=
uld
> > > > 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 th=
at
> > > > into a VMA.
> > > >=20
> > > > 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?
>=20
> Yes, that could certainly be.
>=20
> > 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.
>=20
> 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 g=
et
PFNs which happen to have an associated struct page?  We've gone in enough =
circles...