All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] kvm: x86: potential shift wrapping bug
@ 2014-11-24 12:53 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2014-11-24 12:53 UTC (permalink / raw)
  To: Gleb Natapov
  Cc: Paolo Bonzini, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
	kvm, kernel-janitors

cs.base is declared as a __u64 variable and vector is a u32 so this
causes a static checker warning.  I'm not very familiar with this code
but my understanding is that the user can set "sipi_vector" to any u32
value in kvm_vcpu_ioctl_x86_set_vcpu_events().

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 34c8f94..6608115 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7000,7 +7000,7 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, unsigned int vector)
 
 	kvm_get_segment(vcpu, &cs, VCPU_SREG_CS);
 	cs.selector = vector << 8;
-	cs.base = vector << 12;
+	cs.base = (u64)vector << 12;
 	kvm_set_segment(vcpu, &cs, VCPU_SREG_CS);
 	kvm_rip_write(vcpu, 0);
 }

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [patch] kvm: x86: potential shift wrapping bug
@ 2014-11-24 12:53 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2014-11-24 12:53 UTC (permalink / raw)
  To: Gleb Natapov
  Cc: Paolo Bonzini, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
	kvm, kernel-janitors

cs.base is declared as a __u64 variable and vector is a u32 so this
causes a static checker warning.  I'm not very familiar with this code
but my understanding is that the user can set "sipi_vector" to any u32
value in kvm_vcpu_ioctl_x86_set_vcpu_events().

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 34c8f94..6608115 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7000,7 +7000,7 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, unsigned int vector)
 
 	kvm_get_segment(vcpu, &cs, VCPU_SREG_CS);
 	cs.selector = vector << 8;
-	cs.base = vector << 12;
+	cs.base = (u64)vector << 12;
 	kvm_set_segment(vcpu, &cs, VCPU_SREG_CS);
 	kvm_rip_write(vcpu, 0);
 }

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [patch] kvm: x86: potential shift wrapping bug
  2014-11-24 12:53 ` Dan Carpenter
@ 2014-11-24 13:33   ` Paolo Bonzini
  -1 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2014-11-24 13:33 UTC (permalink / raw)
  To: Dan Carpenter, Gleb Natapov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, kvm, kernel-janitors



On 24/11/2014 13:53, Dan Carpenter wrote:
> cs.base is declared as a __u64 variable and vector is a u32 so this
> causes a static checker warning.  I'm not very familiar with this code
> but my understanding is that the user can set "sipi_vector" to any u32
> value in kvm_vcpu_ioctl_x86_set_vcpu_events().

The user can do so, but it should not set it to any value greater than
255.  So the right fix is to cast to (u8).

Thanks for the report!

Paolo

> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 34c8f94..6608115 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7000,7 +7000,7 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, unsigned int vector)
>  
>  	kvm_get_segment(vcpu, &cs, VCPU_SREG_CS);
>  	cs.selector = vector << 8;
> -	cs.base = vector << 12;
> +	cs.base = (u64)vector << 12;
>  	kvm_set_segment(vcpu, &cs, VCPU_SREG_CS);
>  	kvm_rip_write(vcpu, 0);
>  }
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch] kvm: x86: potential shift wrapping bug
@ 2014-11-24 13:33   ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2014-11-24 13:33 UTC (permalink / raw)
  To: Dan Carpenter, Gleb Natapov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, kvm, kernel-janitors



On 24/11/2014 13:53, Dan Carpenter wrote:
> cs.base is declared as a __u64 variable and vector is a u32 so this
> causes a static checker warning.  I'm not very familiar with this code
> but my understanding is that the user can set "sipi_vector" to any u32
> value in kvm_vcpu_ioctl_x86_set_vcpu_events().

The user can do so, but it should not set it to any value greater than
255.  So the right fix is to cast to (u8).

Thanks for the report!

Paolo

> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 34c8f94..6608115 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7000,7 +7000,7 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, unsigned int vector)
>  
>  	kvm_get_segment(vcpu, &cs, VCPU_SREG_CS);
>  	cs.selector = vector << 8;
> -	cs.base = vector << 12;
> +	cs.base = (u64)vector << 12;
>  	kvm_set_segment(vcpu, &cs, VCPU_SREG_CS);
>  	kvm_rip_write(vcpu, 0);
>  }
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-11-24 13:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-24 12:53 [patch] kvm: x86: potential shift wrapping bug Dan Carpenter
2014-11-24 12:53 ` Dan Carpenter
2014-11-24 13:33 ` Paolo Bonzini
2014-11-24 13:33   ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.