linux-kernel - Re: [Resend Patch] KVM/x86: Fix wrong macro references of X86_CR0_PG_BIT and X86_CR4_PAE_BIT in kvm_valid

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: <20180118183904.3fxeyfuyzlumgtrt@gmail.com>
Date:   Thu, 18 Jan 2018 10:39:04 -0800
From:   Eric Biggers <ebiggers3@...il.com>
To:     Tianyu Lan <lantianyu1986@...il.com>
Cc:     Tianyu Lan <Tianyu.Lan@...rosoft.com>, pbonzini@...hat.com,
        rkrcmar@...hat.com, tglx@...utronix.de, mingo@...hat.com,
        hpa@...or.com, x86@...nel.org, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, jeremi.piotrowski@...il.com
Subject: Re: [Resend Patch] KVM/x86: Fix wrong macro references of
 X86_CR0_PG_BIT and X86_CR4_PAE_BIT in kvm_valid_sregs()

On Tue, Jan 16, 2018 at 05:34:07PM +0800, Tianyu Lan wrote:
> kvm_valid_sregs() should use X86_CR0_PG and X86_CR4_PAE to check bit
> status rather than X86_CR0_PG_BIT and X86_CR4_PAE_BIT. This patch is
> to fix it.
> 
> Fixes: f29810335965a(KVM/x86: Check input paging mode when cs.l is set)
> Reported-by: Jeremi Piotrowski <jeremi.piotrowski@...il.com>
> Cc: Paolo Bonzini <pbonzini@...hat.com>
> Cc: Radim Krčmář <rkrcmar@...hat.com>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@...rosoft.com>
> ---
> Sorry for noise. Missed kvm maillist.
> 
>  arch/x86/kvm/x86.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1cec2c6..c53298d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7496,13 +7496,13 @@ EXPORT_SYMBOL_GPL(kvm_task_switch);
>  
>  int kvm_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
>  {
> -	if ((sregs->efer & EFER_LME) && (sregs->cr0 & X86_CR0_PG_BIT)) {
> +	if ((sregs->efer & EFER_LME) && (sregs->cr0 & X86_CR0_PG)) {
>  		/*
>  		 * When EFER.LME and CR0.PG are set, the processor is in
>  		 * 64-bit mode (though maybe in a 32-bit code segment).
>  		 * CR4.PAE and EFER.LMA must be set.
>  		 */
> -		if (!(sregs->cr4 & X86_CR4_PAE_BIT)
> +		if (!(sregs->cr4 & X86_CR4_PAE)
>  		    || !(sregs->efer & EFER_LMA))
>  			return -EINVAL;
>  	} else {
> -- 
> 2.7.4
> 

I came across this too and was just about to send the exact same patch.  It
looks good to me as long as the bits it's supposed to be checking were correct
in the first place.  Patch title could maybe be shortened a bit, e.g. "KVM/x86:
Fix references to CR0.PG and CR4.PAE in kvm_valid_sregs()".  The "Fixes:" line
is also formatted incorrectly.

Thanks,

Eric