[v2] cpuid: initialize cpuinfo with boot_cpu_data

[PATCH CPU v2] cpuid: initialize cpuinfo with boot_cpu_data

Posted by Norbert Manthey 2 years, 2 months ago

When re-identifying CPU data, we might use uninitialized data when
checking for the cache line property to adapt the cache
alignment. The data that depends on this uninitialized read is
currently not forwarded.

To avoid problems in the future, initialize the data cpuinfo
structure before re-identifying the CPU again.

The trace to hit the uninitialized read reported by Coverity is:

bool recheck_cpu_features(unsigned int cpu)
...
    struct cpuinfo_x86 c;
    ...
    identify_cpu(&c);

void identify_cpu(struct cpuinfo_x86 *c)
...
    generic_identify(c)

static void generic_identify(struct cpuinfo_x86 *c)
...
        if (this_cpu->c_early_init)
                this_cpu->c_early_init(c); // which is early_init_intel

static void early_init_intel(struct cpuinfo_x86 *c)
...
    if (c->x86 == 15 && c->x86_cache_alignment == 64)
        c->x86_cache_alignment = 128;

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

---
 xen/arch/x86/cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -609,7 +609,7 @@ void __init init_guest_cpuid(void)
 bool recheck_cpu_features(unsigned int cpu)
 {
     bool okay = true;
-    struct cpuinfo_x86 c;
+    struct cpuinfo_x86 c = {0};
     const struct cpuinfo_x86 *bsp = &boot_cpu_data;
     unsigned int i;
 
-- 
2.17.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

Re: [PATCH CPU v2] cpuid: initialize cpuinfo with boot_cpu_data

Posted by Jan Beulich 2 years, 2 months ago

On 11.02.2022 16:25, Norbert Manthey wrote:
> When re-identifying CPU data, we might use uninitialized data when
> checking for the cache line property to adapt the cache
> alignment. The data that depends on this uninitialized read is
> currently not forwarded.
> 
> To avoid problems in the future, initialize the data cpuinfo
> structure before re-identifying the CPU again.
> 
> The trace to hit the uninitialized read reported by Coverity is:
> 
> bool recheck_cpu_features(unsigned int cpu)
> ...
>     struct cpuinfo_x86 c;
>     ...
>     identify_cpu(&c);
> 
> void identify_cpu(struct cpuinfo_x86 *c)
> ...
>     generic_identify(c)
> 
> static void generic_identify(struct cpuinfo_x86 *c)
> ...
>         if (this_cpu->c_early_init)
>                 this_cpu->c_early_init(c); // which is early_init_intel
> 
> static void early_init_intel(struct cpuinfo_x86 *c)
> ...
>     if (c->x86 == 15 && c->x86_cache_alignment == 64)
>         c->x86_cache_alignment = 128;
> 
> This bug was discovered and resolved using Coverity Static Analysis
> Security Testing (SAST) by Synopsys, Inc.
> 
> Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

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