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 <christian.koenig@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...