linux-kernel - Re: [PATCH 5.4] KVM: x86/mmu: do compare-and-exchange of gPTE via the user address

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]

Date:   Mon, 4 Apr 2022 16:02:40 +0200
From:   Greg KH <greg@...ah.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        stable@...r.kernel.org, Qiuhao Li <qiuhao@...ec.org>,
        Gaoning Pan <pgn@....edu.cn>, Yongkang Jia <kangel@....edu.cn>,
        syzbot+6cde2282daa792c49ab8@...kaller.appspotmail.com,
        Tadeusz Struk <tadeusz.struk@...aro.org>,
        Maxim Levitsky <mlevitsk@...hat.com>
Subject: Re: [PATCH 5.4] KVM: x86/mmu: do compare-and-exchange of gPTE via
 the user address

On Mon, Apr 04, 2022 at 09:41:41AM -0400, Paolo Bonzini wrote:
> commit 2a8859f373b0a86f0ece8ec8312607eacf12485d upstream.
> 
> FNAME(cmpxchg_gpte) is an inefficient mess.  It is at least decent if it
> can go through get_user_pages_fast(), but if it cannot then it tries to
> use memremap(); that is not just terribly slow, it is also wrong because
> it assumes that the VM_PFNMAP VMA is contiguous.
> 
> The right way to do it would be to do the same thing as
> hva_to_pfn_remapped() does since commit add6a0cd1c5b ("KVM: MMU: try to
> fix up page faults before giving up", 2016-07-05), using follow_pte()
> and fixup_user_fault() to determine the correct address to use for
> memremap().  To do this, one could for example extract hva_to_pfn()
> for use outside virt/kvm/kvm_main.c.  But really there is no reason to
> do that either, because there is already a perfectly valid address to
> do the cmpxchg() on, only it is a userspace address.  That means doing
> user_access_begin()/user_access_end() and writing the code in assembly
> to handle any exception correctly.  Worse, the guest PTE can be 8-byte
> even on i686 so there is the extra complication of using cmpxchg8b to
> account for.  But at least it is an efficient mess.
> 
> Reported-by: Qiuhao Li <qiuhao@...ec.org>
> Reported-by: Gaoning Pan <pgn@....edu.cn>
> Reported-by: Yongkang Jia <kangel@....edu.cn>
> Reported-by: syzbot+6cde2282daa792c49ab8@...kaller.appspotmail.com
> Debugged-by: Tadeusz Struk <tadeusz.struk@...aro.org>
> Tested-by: Maxim Levitsky <mlevitsk@...hat.com>
> Cc: stable@...r.kernel.org
> Fixes: bd53cb35a3e9 ("X86/KVM: Handle PFNs outside of kernel reach when touching GPTEs")
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
>  arch/x86/kvm/paging_tmpl.h | 77 ++++++++++++++++++--------------------
>  1 file changed, 37 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> index 97b21e7fd013..13b5c424adb2 100644
> --- a/arch/x86/kvm/paging_tmpl.h
> +++ b/arch/x86/kvm/paging_tmpl.h
> @@ -34,9 +34,8 @@
>  	#define PT_HAVE_ACCESSED_DIRTY(mmu) true
>  	#ifdef CONFIG_X86_64
>  	#define PT_MAX_FULL_LEVELS 4
> -	#define CMPXCHG cmpxchg
> +	#define CMPXCHG "cmpxchgq"
>  	#else
> -	#define CMPXCHG cmpxchg64
>  	#define PT_MAX_FULL_LEVELS 2
>  	#endif
>  #elif PTTYPE == 32
> @@ -52,7 +51,7 @@

This chunk does not apply, are you sure you made this against 5.4.y?

thanks,

greg k-h