[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] x86/hvmloader: fix usage of NULL with cpuid_count()



The commit that added support for retrieving the APIC IDs from the APs
introduced several usages of cpuid() with NULL parameters, which is not
handled by the underlying implementation.  For GCC I expect this results in
writes to the physical address at 0, however for Clang the generated code
in smp.o is:

tools/firmware/hvmloader/smp.o: file format elf32-i386

Disassembly of section .text:

00000000 <smp_initialise>:
       0: 55                            pushl   %ebp
       1: 89 e5                         movl    %esp, %ebp
       3: 53                            pushl   %ebx
       4: 31 c0                         xorl    %eax, %eax
       6: 31 c9                         xorl    %ecx, %ecx
       8: 0f a2                         cpuid

Showing the usage of a NULL pointer results in undefined behavior, and
clang refusing to generate further code after it.

Fix by using a temporary variable in cpuid_count() in place for any NULL
parameter.

Fixes: 9ad0db58c7e2 ('tools/hvmloader: Retrieve APIC IDs from the APs 
themselves')
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
Could also be fixed by using the temporary variable in the call sites,
however that's more code in the call sites at the expense of less checking.
I don't think the extra NULL check logic in cpuid_count() is that bad.

Overall the solution proposed in this patch is safer going forward, as it
prevent issues like this from being introduced in the first place.
---
 tools/firmware/hvmloader/util.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index 644450c51ceb..765a013ddd9e 100644
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -190,6 +190,17 @@ static inline void cpuid_count(
     uint32_t *ecx,
     uint32_t *edx)
 {
+    uint32_t tmp;
+
+    if ( !eax )
+        eax = &tmp;
+    if ( !ebx )
+        ebx = &tmp;
+    if ( !ecx )
+        ecx = &tmp;
+    if ( !edx )
+        edx = &tmp;
+
     asm volatile ( "cpuid"
                    : "=a" (*eax), "=b" (*ebx), "=c" (*ecx), "=d" (*edx)
                    : "a" (leaf), "c" (subleaf) );
-- 
2.48.1




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.