[PATCH 1/3] xen/common: Avoid undefined behaviour by shifting into a sign bit

[PATCH 1/3] xen/common: Avoid undefined behaviour by shifting into a sign bit

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
---
 xen/common/domctl.c       | 2 +-
 xen/common/xmalloc_tlsf.c | 4 ++--
 xen/include/xen/sched.h   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 8f25131..cf7928c 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -188,7 +188,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
         (d->controller_pause_count > 0  ? XEN_DOMINF_paused    : 0) |
         (d->debugger_attached           ? XEN_DOMINF_debugged  : 0) |
         (d->is_xenstore                 ? XEN_DOMINF_xs_domain : 0) |
-        d->shutdown_code << XEN_DOMINF_shutdownshift;
+        (unsigned int)d->shutdown_code << XEN_DOMINF_shutdownshift;
 
     switch ( d->guest_type )
     {
diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
index b13317e..6c1b882 100644
--- a/xen/common/xmalloc_tlsf.c
+++ b/xen/common/xmalloc_tlsf.c
@@ -177,7 +177,7 @@ static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl)
 static inline struct bhdr *FIND_SUITABLE_BLOCK(struct xmem_pool *p, int *fl,
                                                int *sl)
 {
-    u32 tmp = p->sl_bitmap[*fl] & (~0 << *sl);
+    u32 tmp = p->sl_bitmap[*fl] & (~0u << *sl);
     struct bhdr *b = NULL;
 
     if ( tmp )
@@ -187,7 +187,7 @@ static inline struct bhdr *FIND_SUITABLE_BLOCK(struct xmem_pool *p, int *fl,
     }
     else
     {
-        *fl = ffs(p->fl_bitmap & (~0 << (*fl + 1))) - 1;
+        *fl = ffs(p->fl_bitmap & (~0u << (*fl + 1))) - 1;
         if ( likely(*fl > 0) )
         {
             *sl = ffs(p->sl_bitmap[*fl]) - 1;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 888bc19..bb4ee4e 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -483,7 +483,7 @@ extern struct vcpu *idle_vcpu[NR_CPUS];
 #define is_idle_domain(d) ((d)->domain_id == DOMID_IDLE)
 #define is_idle_vcpu(v)   (is_idle_domain((v)->domain))
 
-#define DOMAIN_DESTROYED (1<<31) /* assumes atomic_t is >= 32 bits */
+#define DOMAIN_DESTROYED (1u << 31) /* assumes atomic_t is >= 32 bits */
 #define put_domain(_d) \
   if ( atomic_dec_and_test(&(_d)->refcnt) ) domain_destroy(_d)
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 2/3] xen/x86: Avoid undefined behaviour by shifting into a sign bit

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/apic.c           | 2 +-
 xen/arch/x86/cpu/common.c     | 2 +-
 xen/arch/x86/x86_64/traps.c   | 2 +-
 xen/include/asm-x86/apicdef.h | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index f3727cd..3fb9a82 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -571,7 +571,7 @@ void setup_local_APIC(void)
     for (i = APIC_ISR_NR - 1; i >= 0; i--) {
         value = apic_read(APIC_ISR + i*0x10);
         for (j = 31; j >= 0; j--) {
-            if (value & (1<<j))
+            if (value & (1u << j))
                 ack_APIC_irq();
         }
     }
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 2c47589..3336908 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -476,7 +476,7 @@ void detect_extended_topology(struct cpuinfo_x86 *c)
 		sub_index++;
 	} while ( LEAFB_SUBTYPE(ecx) != INVALID_TYPE );
 
-	core_select_mask = (~(-1 << core_plus_mask_width)) >> ht_mask_width;
+	core_select_mask = (~(-1u << core_plus_mask_width)) >> ht_mask_width;
 
 	c->cpu_core_id = phys_pkg_id(initial_apicid, ht_mask_width)
 		& core_select_mask;
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 19f58a1..1b1b61a 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -414,7 +414,7 @@ void subarch_percpu_traps_init(void)
     unmap_domain_page(stub_page);
 
     /* Common SYSCALL parameters. */
-    wrmsr(MSR_STAR, 0, (FLAT_RING3_CS32<<16) | __HYPERVISOR_CS);
+    wrmsr(MSR_STAR, 0, ((unsigned)FLAT_RING3_CS32 << 16) | __HYPERVISOR_CS);
     wrmsr(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK, 0U);
 }
 
diff --git a/xen/include/asm-x86/apicdef.h b/xen/include/asm-x86/apicdef.h
index 8752287..da7f4d3 100644
--- a/xen/include/asm-x86/apicdef.h
+++ b/xen/include/asm-x86/apicdef.h
@@ -30,7 +30,7 @@
 #define			APIC_EIO_ACK		0x0		/* Write this to the EOI register */
 #define		APIC_RRR	0xC0
 #define		APIC_LDR	0xD0
-#define			APIC_LDR_MASK		(0xFF<<24)
+#define			APIC_LDR_MASK		(0xFFu<<24)
 #define			GET_xAPIC_LOGICAL_ID(x)	(((x)>>24)&0xFF)
 #define			SET_xAPIC_LOGICAL_ID(x)	(((x)<<24))
 #define			APIC_ALL_CPUS		0xFF
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 3/3] x86/microcode: Avoid undefined behaviour from signed integer overflow

[Andrew Cooper]

The checksum should be calculated using unsigned 32bit integers, as it is
intended to overflow and end at 0.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
---
 xen/arch/x86/microcode_intel.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
index 6949c25..5b411b4 100644
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode_intel.c
@@ -143,7 +143,8 @@ static int microcode_sanity_check(void *mc)
     struct extended_sigtable *ext_header = NULL;
     struct extended_signature *ext_sig;
     unsigned long total_size, data_size, ext_table_size;
-    int sum, orig_sum, ext_sigcount = 0, i;
+    uint32_t sum, orig_sum;
+    int ext_sigcount = 0, i;
 
     total_size = get_totalsize(mc_header);
     data_size = get_datasize(mc_header);
@@ -201,7 +202,7 @@ static int microcode_sanity_check(void *mc)
     orig_sum = 0;
     i = (MC_HEADER_SIZE + data_size) / DWSIZE;
     while ( i-- )
-        orig_sum += ((int *)mc)[i];
+        orig_sum += ((uint32_t *)mc)[i];
     if ( orig_sum )
     {
         printk(KERN_ERR "microcode: aborting, bad checksum\n");
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] xen/common: Avoid undefined behaviour by shifting into a sign bit

[Jan Beulich]

>>> On 05.08.16 at 15:50, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -188,7 +188,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
>          (d->controller_pause_count > 0  ? XEN_DOMINF_paused    : 0) |
>          (d->debugger_attached           ? XEN_DOMINF_debugged  : 0) |
>          (d->is_xenstore                 ? XEN_DOMINF_xs_domain : 0) |
> -        d->shutdown_code << XEN_DOMINF_shutdownshift;
> +        (unsigned int)d->shutdown_code << XEN_DOMINF_shutdownshift;

Is adding a cast here really the most suitable fix? The only two places
shutdown_code gets set (besides the -1 initialization) have their right
side a u8. Nothing ever checks for the value being negative (there are
just two -1 checks), so converting the field to u8 or unsigned int (and
using a sentinel different from -1) should both work, avoiding the need
for a cast.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/3] xen/x86: Avoid undefined behaviour by shifting into a sign bit

[Jan Beulich]

>>> On 05.08.16 at 15:50, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -476,7 +476,7 @@ void detect_extended_topology(struct cpuinfo_x86 *c)
>  		sub_index++;
>  	} while ( LEAFB_SUBTYPE(ecx) != INVALID_TYPE );
>  
> -	core_select_mask = (~(-1 << core_plus_mask_width)) >> ht_mask_width;
> +	core_select_mask = (~(-1u << core_plus_mask_width)) >> ht_mask_width;

-1u is kind of bogus; could I talk you into using ~0u instead?

With that
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] xen/common: Avoid undefined behaviour by shifting into a sign bit

[George Dunlap]

On 05/08/16 15:04, Jan Beulich wrote:
>>>> On 05.08.16 at 15:50, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -188,7 +188,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
>>          (d->controller_pause_count > 0  ? XEN_DOMINF_paused    : 0) |
>>          (d->debugger_attached           ? XEN_DOMINF_debugged  : 0) |
>>          (d->is_xenstore                 ? XEN_DOMINF_xs_domain : 0) |
>> -        d->shutdown_code << XEN_DOMINF_shutdownshift;
>> +        (unsigned int)d->shutdown_code << XEN_DOMINF_shutdownshift;
> 
> Is adding a cast here really the most suitable fix? The only two places
> shutdown_code gets set (besides the -1 initialization) have their right
> side a u8. Nothing ever checks for the value being negative (there are
> just two -1 checks), so converting the field to u8 or unsigned int (and
> using a sentinel different from -1) should both work, avoiding the need
> for a cast.

This seems sensible if possible.

The other bits:

Reviewed-by: George Dunlap <george.dunlap@citrix.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/3] x86/microcode: Avoid undefined behaviour from signed integer overflow

[Jan Beulich]

>>> On 05.08.16 at 15:50, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/microcode_intel.c
> +++ b/xen/arch/x86/microcode_intel.c
> @@ -143,7 +143,8 @@ static int microcode_sanity_check(void *mc)
>      struct extended_sigtable *ext_header = NULL;
>      struct extended_signature *ext_sig;
>      unsigned long total_size, data_size, ext_table_size;
> -    int sum, orig_sum, ext_sigcount = 0, i;
> +    uint32_t sum, orig_sum;
> +    int ext_sigcount = 0, i;

Pretty clearly these other two want to be unsigned int? If you agree,
Reviewed-by: Jan Beulich <jbeulich@suse.com>.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH] xen/common: Avoid undefined behaviour by shifting into a sign bit

[Andrew Cooper]

For d->shutdown_code, change the field to being unsigned and using an unsigned
sentinel.  The sentinal needs to be distinguishable from any value
representable in a u8.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>

v2:
 * Change d->shutdown_code to being unsigned.
---
 xen/common/domain.c       | 6 +++---
 xen/common/schedule.c     | 2 +-
 xen/common/xmalloc_tlsf.c | 4 ++--
 xen/include/xen/sched.h   | 5 +++--
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 339ee56..a8804e4 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -293,7 +293,7 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
     d->auto_node_affinity = 1;
 
     spin_lock_init(&d->shutdown_lock);
-    d->shutdown_code = -1;
+    d->shutdown_code = SHUTDOWN_CODE_INVALID;
 
     spin_lock_init(&d->pbuf_lock);
 
@@ -695,7 +695,7 @@ void domain_shutdown(struct domain *d, u8 reason)
 
     spin_lock(&d->shutdown_lock);
 
-    if ( d->shutdown_code == -1 )
+    if ( d->shutdown_code == SHUTDOWN_CODE_INVALID )
         d->shutdown_code = reason;
     reason = d->shutdown_code;
 
@@ -742,7 +742,7 @@ void domain_resume(struct domain *d)
     spin_lock(&d->shutdown_lock);
 
     d->is_shutting_down = d->is_shut_down = 0;
-    d->shutdown_code = -1;
+    d->shutdown_code = SHUTDOWN_CODE_INVALID;
 
     for_each_vcpu ( d, v )
     {
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 852f840..32a300f 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1120,7 +1120,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
                  d->domain_id, current->vcpu_id, sched_shutdown.reason);
 
         spin_lock(&d->shutdown_lock);
-        if ( d->shutdown_code == -1 )
+        if ( d->shutdown_code == SHUTDOWN_CODE_INVALID )
             d->shutdown_code = (u8)sched_shutdown.reason;
         spin_unlock(&d->shutdown_lock);
 
diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
index b13317e..6c1b882 100644
--- a/xen/common/xmalloc_tlsf.c
+++ b/xen/common/xmalloc_tlsf.c
@@ -177,7 +177,7 @@ static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl)
 static inline struct bhdr *FIND_SUITABLE_BLOCK(struct xmem_pool *p, int *fl,
                                                int *sl)
 {
-    u32 tmp = p->sl_bitmap[*fl] & (~0 << *sl);
+    u32 tmp = p->sl_bitmap[*fl] & (~0u << *sl);
     struct bhdr *b = NULL;
 
     if ( tmp )
@@ -187,7 +187,7 @@ static inline struct bhdr *FIND_SUITABLE_BLOCK(struct xmem_pool *p, int *fl,
     }
     else
     {
-        *fl = ffs(p->fl_bitmap & (~0 << (*fl + 1))) - 1;
+        *fl = ffs(p->fl_bitmap & (~0u << (*fl + 1))) - 1;
         if ( likely(*fl > 0) )
         {
             *sl = ffs(p->sl_bitmap[*fl]) - 1;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 888bc19..2f9c15f 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -404,7 +404,8 @@ struct domain
     spinlock_t       shutdown_lock;
     bool_t           is_shutting_down; /* in process of shutting down? */
     bool_t           is_shut_down;     /* fully shut down? */
-    int              shutdown_code;
+#define SHUTDOWN_CODE_INVALID ~0u
+    unsigned int     shutdown_code;
 
     /* If this is not 0, send suspend notification here instead of
      * raising DOM_EXC */
@@ -483,7 +484,7 @@ extern struct vcpu *idle_vcpu[NR_CPUS];
 #define is_idle_domain(d) ((d)->domain_id == DOMID_IDLE)
 #define is_idle_vcpu(v)   (is_idle_domain((v)->domain))
 
-#define DOMAIN_DESTROYED (1<<31) /* assumes atomic_t is >= 32 bits */
+#define DOMAIN_DESTROYED (1u << 31) /* assumes atomic_t is >= 32 bits */
 #define put_domain(_d) \
   if ( atomic_dec_and_test(&(_d)->refcnt) ) domain_destroy(_d)
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH] xen/x86: Avoid undefined behaviour by shifting into a sign bit

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

v2:
 * Use ~0u as opposed to -1u
---
 xen/arch/x86/apic.c           | 2 +-
 xen/arch/x86/cpu/common.c     | 2 +-
 xen/arch/x86/x86_64/traps.c   | 2 +-
 xen/include/asm-x86/apicdef.h | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index f3727cd..3fb9a82 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -571,7 +571,7 @@ void setup_local_APIC(void)
     for (i = APIC_ISR_NR - 1; i >= 0; i--) {
         value = apic_read(APIC_ISR + i*0x10);
         for (j = 31; j >= 0; j--) {
-            if (value & (1<<j))
+            if (value & (1u << j))
                 ack_APIC_irq();
         }
     }
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 2c47589..577a01f 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -476,7 +476,7 @@ void detect_extended_topology(struct cpuinfo_x86 *c)
 		sub_index++;
 	} while ( LEAFB_SUBTYPE(ecx) != INVALID_TYPE );
 
-	core_select_mask = (~(-1 << core_plus_mask_width)) >> ht_mask_width;
+	core_select_mask = (~(~0u << core_plus_mask_width)) >> ht_mask_width;
 
 	c->cpu_core_id = phys_pkg_id(initial_apicid, ht_mask_width)
 		& core_select_mask;
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 19f58a1..2d8ecf5 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -414,7 +414,7 @@ void subarch_percpu_traps_init(void)
     unmap_domain_page(stub_page);
 
     /* Common SYSCALL parameters. */
-    wrmsr(MSR_STAR, 0, (FLAT_RING3_CS32<<16) | __HYPERVISOR_CS);
+    wrmsr(MSR_STAR, 0, ((unsigned int)FLAT_RING3_CS32 << 16) | __HYPERVISOR_CS);
     wrmsr(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK, 0U);
 }
 
diff --git a/xen/include/asm-x86/apicdef.h b/xen/include/asm-x86/apicdef.h
index 8752287..da7f4d3 100644
--- a/xen/include/asm-x86/apicdef.h
+++ b/xen/include/asm-x86/apicdef.h
@@ -30,7 +30,7 @@
 #define			APIC_EIO_ACK		0x0		/* Write this to the EOI register */
 #define		APIC_RRR	0xC0
 #define		APIC_LDR	0xD0
-#define			APIC_LDR_MASK		(0xFF<<24)
+#define			APIC_LDR_MASK		(0xFFu<<24)
 #define			GET_xAPIC_LOGICAL_ID(x)	(((x)>>24)&0xFF)
 #define			SET_xAPIC_LOGICAL_ID(x)	(((x)<<24))
 #define			APIC_ALL_CPUS		0xFF
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH] x86/microcode: Avoid undefined behaviour from signed integer overflow

[Andrew Cooper]

The checksums should be calculated using unsigned 32bit integers, as they are
intended to overflow and end at 0.  Replace some other signed integers with
unsigned ones, to avoid mixed-sign comparisons.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Jun Nakajima <jun.nakajima@intel.com>

v2:
 * Include the extended checksum
 * Swap more integers to being unsigned
---
 xen/arch/x86/microcode_intel.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
index 6949c25..93d9d0f 100644
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode_intel.c
@@ -143,7 +143,8 @@ static int microcode_sanity_check(void *mc)
     struct extended_sigtable *ext_header = NULL;
     struct extended_signature *ext_sig;
     unsigned long total_size, data_size, ext_table_size;
-    int sum, orig_sum, ext_sigcount = 0, i;
+    unsigned int ext_sigcount = 0, i;
+    uint32_t sum, orig_sum;
 
     total_size = get_totalsize(mc_header);
     data_size = get_datasize(mc_header);
@@ -183,8 +184,8 @@ static int microcode_sanity_check(void *mc)
     /* check extended table checksum */
     if ( ext_table_size )
     {
-        int ext_table_sum = 0;
-        int *ext_tablep = (int *)ext_header;
+        uint32_t ext_table_sum = 0;
+        uint32_t *ext_tablep = (uint32_t *)ext_header;
 
         i = ext_table_size / DWSIZE;
         while ( i-- )
@@ -201,7 +202,7 @@ static int microcode_sanity_check(void *mc)
     orig_sum = 0;
     i = (MC_HEADER_SIZE + data_size) / DWSIZE;
     while ( i-- )
-        orig_sum += ((int *)mc)[i];
+        orig_sum += ((uint32_t *)mc)[i];
     if ( orig_sum )
     {
         printk(KERN_ERR "microcode: aborting, bad checksum\n");
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] xen/common: Avoid undefined behaviour by shifting into a sign bit

[Jan Beulich]

>>> On 09.08.16 at 14:48, <andrew.cooper3@citrix.com> wrote:
> For d->shutdown_code, change the field to being unsigned and using an unsigned
> sentinel.  The sentinal needs to be distinguishable from any value
> representable in a u8.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: George Dunlap <george.dunlap@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/3] x86/microcode: Avoid undefined behaviour from signed integer overflow

[Tian, Kevin]

> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Friday, August 05, 2016 9:50 PM
> To: Xen-devel
> Cc: Andrew Cooper; Jan Beulich; Tian, Kevin; Nakajima, Jun
> Subject: [PATCH 3/3] x86/microcode: Avoid undefined behaviour from signed integer
> overflow
> 
> The checksum should be calculated using unsigned 32bit integers, as it is
> intended to overflow and end at 0.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel