linux-kernel - Re: [PATCH v7 03/14] KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP

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: <20200323171353.GL127076@xz-x1>
Date:   Mon, 23 Mar 2020 13:13:53 -0400
From:   Peter Xu <peterx@...hat.com>
To:     Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        "Michael S . Tsirkin" <mst@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Yan Zhao <yan.y.zhao@...el.com>,
        "Dr . David Alan Gilbert" <dgilbert@...hat.com>,
        Christophe de Dinechin <dinechin@...hat.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Kevin Tian <kevin.tian@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v7 03/14] KVM: X86: Don't track dirty for
 KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]

On Mon, Mar 23, 2020 at 09:55:51AM -0700, Sean Christopherson wrote:
> On Mon, Mar 23, 2020 at 12:26:17PM -0400, Peter Xu wrote:
> > On Mon, Mar 23, 2020 at 08:42:16AM -0700, Sean Christopherson wrote:
> > > > > Regarding the HVA, it's a bit confusing saying that it's guaranteed to be
> > > > > valid, and then contradicting that in the second clause.  Maybe something
> > > > > like this to explain the GPA->HVA is guaranteed to be valid, but the
> > > > > HVA->HPA is not.
> > > > >  
> > > > > /*
> > > > >  * before use.  Note, KVM internal memory slots are guaranteed to remain valid
> > > > >  * and unchanged until the VM is destroyed, i.e. the GPA->HVA translation will
> > > > >  * not change.  However, the HVA is a user address, i.e. its accessibility is
> > > > >  * not guaranteed, and must be accessed via __copy_{to,from}_user().
> > > > >  */
> > > > 
> > > > Sure I can switch to this, though note that I still think the GPA->HVA
> > > > is not guaranteed logically because the userspace can unmap any HVA it
> > > > wants..
> > > 
> > > You're conflating the GPA->HVA translation with the validity of the HVA,
> > > i.e. the HVA->HPA and/or HVA->VMA translation/association.  GPA->HVA is
> > > guaranteed because userspace doesn't have access to the memslot which
> > > defines that transation.
> > 
> > Yes I completely agree if you mean the pure mapping of GPA->HVA.
> > 
> > I think it's a matter of how to define the "valid" when you say
> > "guaranteed to remain valid", because I don't think the mapping is
> > still valid from the most strict sense if e.g. the backing HVA does
> > not exist any more for that GPA->HVA mapping, then the memslot won't
> > be anything useful.
> 
> Yes.  That's why my proposed comment is worded to state that the _memslot_
> will remain valid.  It deliberately avoids mentioning "valid HVA".

OK, I see the point.  I did re-read the two versions again, I agree
yours is better, which I'll replace with.  Thanks!

-- 
Peter Xu