[Resend Patch] KVM/x86: Fix wrong macro references of X86_CR0_PG_BIT and X86_CR4_PAE_BIT in kvm_valid_sregs()

[Resend Patch] KVM/x86: Fix wrong macro references of X86_CR0_PG_BIT and X86_CR4_PAE_BIT in kvm_valid_sregs()

[Tianyu Lan]

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@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.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

Re: [Resend Patch] KVM/x86: Fix wrong macro references of X86_CR0_PG_BIT and X86_CR4_PAE_BIT in kvm_valid_sregs()

[Eric Biggers]

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@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.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

Re: [Resend Patch] KVM/x86: Fix wrong macro references of X86_CR0_PG_BIT and X86_CR4_PAE_BIT in kvm_valid_sregs()

[Lan Tianyu]

Hi Eric:
	Great thanks for your review.
On Thu, Jan 18, 2018 at 10:39:04AM -0800, Eric Biggers wrote:
> 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@gmail.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Radim Krčmář <rkrcmar@redhat.com>
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.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.

That will be better and will update.

> 
> Thanks,
> 
> Eric