linux-kernel - Re: [PATCH v7 036/102] KVM: x86/mmu: Allow non-zero value for non-present SPTE

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d7f60ee5e2bdd72e8b1fbcabb753170167674eee.camel@intel.com>
Date:   Wed, 20 Jul 2022 15:12:16 +1200
From:   Kai Huang <kai.huang@...el.com>
To:     Isaku Yamahata <isaku.yamahata@...il.com>, isaku.yamahata@...el.com
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Yuan Yao <yuan.yao@...ux.intel.com>
Subject: Re: [PATCH v7 036/102] KVM: x86/mmu: Allow non-zero value for
 non-present SPTE

> --- a/arch/x86/kvm/mmu/spte.c
> +++ b/arch/x86/kvm/mmu/spte.c
> @@ -36,6 +36,9 @@ u64 __read_mostly shadow_present_mask;
>  u64 __read_mostly shadow_me_value;
>  u64 __read_mostly shadow_me_mask;
>  u64 __read_mostly shadow_acc_track_mask;
> +#ifdef CONFIG_X86_64
> +u64 __read_mostly shadow_nonpresent_value;
> +#endif

Is this ever used?

>  
>  u64 __read_mostly shadow_nonpresent_or_rsvd_mask;
>  u64 __read_mostly shadow_nonpresent_or_rsvd_lower_gfn_mask;
> @@ -360,7 +363,7 @@ void kvm_mmu_set_mmio_spte_mask(u64 mmio_value, u64 mmio_mask, u64 access_mask)
>  	 * not set any RWX bits.
>  	 */
>  	if (WARN_ON((mmio_value & mmio_mask) != mmio_value) ||
> -	    WARN_ON(mmio_value && (REMOVED_SPTE & mmio_mask) == mmio_value))
> +	    WARN_ON(mmio_value && (__REMOVED_SPTE & mmio_mask) == mmio_value))
>  		mmio_value = 0;

This chunk doesn't look right, or necessary.  We need mmio_mask/mmio_value which
causes EPT violation but with "suppress #VE" bit clear.  

So, actually, we want to make sure SHADOW_NONPRESENT_VALUE is *NOT* in mmio_mask
and mmio_value.  Using (REMOVED_SPTE & mmio_mask) == mmio_value can actually
ensure SHADOW_NONPRESENT_VALUE is never set in MMIO spte, correct?  So I think
using REMOVED_SPTE is fine.

Or maybe additionally adding a explicit check is even better:

	if (WARN_ON(mmio_mask & SHADOW_NONPRESENT_VALUE))
		mmio_value = 0;

But this change maybe should be in another patch which deals setting up per-VM
mmio_mask/mmio_value anyway.  This patch, instead, focuses on allowing non-zero
value for non-present SPTE.