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

[PATCH 09/10] x86/ucode: Drop ucode_mod and ucode_blob



Both are used to pass information from early_microcode_load() to
microcode_init_cache(), and both constitute use-after-free's in certain cases.

 * ucode_mod is a copy of the module_t identified by `ucode=$n`.  Except it's
   a copy from prior to Xen relocating the modules.  microcode_init_cache()
   might happen to find the data still intact in it's old location, but it
   might be an arbitrary part of some other module.

 * ucode_blob is a stashed pointer to a bootstrap_map() which becomes invalid
   the moment control returns to __start_xen(), where other logic is free to
   to make temporary mappings.  This was even noticed and
   microcode_init_cache() adjusted to "fix" the stashed pointers.

The information which should be passed between these two functions is which
module to look in.  Introduce early_mod_idx for this purpose.  opt_scan can be
reused to distinguish between CPIO archives and raw containers.

Notably this means microcode_init_cache() doesn't need to scan all modules any
more; we know exactly which one to look in.  However, we do re-parse the CPIO
header for simplicity.

Furthermore, microcode_init_cache(), being a presmp_initcall does not need to
use bootstrap_map() to access modules; it can use mfn_to_virt() directly,
which avoids needing to funnel exit paths through bootstrap_unmap().

Fold microcode_scan_module() into what is now it's single caller.  It operates
on the function-wide idx/data/size state which allows for a unified "found"
path irrespective of module selection method.

This resolves all issues to do with holding pointers (physical or virtual)
across returning to __start_xen().

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
---
 xen/arch/x86/cpu/microcode/core.c | 172 ++++++++++++++++--------------
 1 file changed, 89 insertions(+), 83 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/core.c 
b/xen/arch/x86/cpu/microcode/core.c
index 28cfeab75a81..591c13ad91fb 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -28,7 +28,6 @@
 #include <xen/err.h>
 #include <xen/guest_access.h>
 #include <xen/init.h>
-#include <xen/multiboot.h>
 #include <xen/param.h>
 #include <xen/spinlock.h>
 #include <xen/stop_machine.h>
@@ -59,7 +58,6 @@
  */
 #define MICROCODE_UPDATE_TIMEOUT_US 1000000
 
-static module_t __initdata ucode_mod;
 static bool __initdata ucode_mod_forced;
 static unsigned int nr_cores;
 
@@ -79,24 +77,11 @@ static enum {
     LOADING_EXIT,
 } loading_state;
 
-/*
- * If we scan the initramfs.cpio for the early microcode code
- * and find it, then 'ucode_blob' will contain the pointer
- * and the size of said blob. It is allocated from Xen's heap
- * memory.
- */
-struct ucode_mod_blob {
-    const void *data;
-    size_t size;
-};
-
 struct patch_with_flags {
     unsigned int flags;
     const struct microcode_patch *patch;
 };
 
-static struct ucode_mod_blob __initdata ucode_blob;
-
 /* By default, ucode loading is done in NMI handler */
 static bool ucode_in_nmi = true;
 
@@ -172,46 +157,6 @@ custom_param("ucode", parse_ucode);
 
 static struct microcode_ops __ro_after_init ucode_ops;
 
-static void __init microcode_scan_module(struct boot_info *bi)
-{
-    uint64_t *_blob_start;
-    unsigned long _blob_size;
-    struct cpio_data cd;
-    int i;
-
-    ucode_blob.size = 0;
-    if ( !opt_scan )
-        return;
-
-    /*
-     * Try all modules and see whichever could be the microcode blob.
-     */
-    for ( i = 1 /* Ignore dom0 kernel */; i < bi->nr_modules; i++ )
-    {
-        if ( !test_bit(i, bi->module_map) )
-            continue;
-
-        _blob_start = bootstrap_map(bi->mods[i].mod);
-        _blob_size = bi->mods[i].mod->mod_end;
-        if ( !_blob_start )
-        {
-            printk("Could not map multiboot module #%d (size: %ld)\n",
-                   i, _blob_size);
-            continue;
-        }
-        cd.data = NULL;
-        cd.size = 0;
-        cd = find_cpio_data(ucode_ops.cpio_path, _blob_start, _blob_size);
-        if ( cd.data )
-        {
-            ucode_blob.size = cd.size;
-            ucode_blob.data = cd.data;
-            break;
-        }
-        bootstrap_unmap();
-    }
-}
-
 static DEFINE_SPINLOCK(microcode_mutex);
 
 DEFINE_PER_CPU(struct cpu_signature, cpu_sig);
@@ -789,23 +734,47 @@ static int __init early_update_cache(const void *data, 
size_t len)
     return rc;
 }
 
+/*
+ * Set by early_microcode_load() to indicate where it found microcode, so
+ * microcode_init_cache() can find it again and initalise the cache.  opt_scan
+ * tells us whether we're looking for a raw container or CPIO archive.
+ */
+static int __initdata early_mod_idx = -1;
+
 static int __init cf_check microcode_init_cache(void)
 {
     struct boot_info *bi = &xen_boot_info;
+    void *data;
+    size_t size;
     int rc = 0;
 
-    if ( !ucode_ops.apply_microcode )
-        return -ENODEV;
+    if ( early_mod_idx < 0 )
+        /* early_microcode_load() didn't leave us any work to do. */
+        return 0;
+
+    size = bi->mods[early_mod_idx].mod->mod_end;
+    data = __mfn_to_virt(bi->mods[early_mod_idx].mod->mod_start);
 
+    /*
+     * If opt_scan is set, we're looking for a CPIO archive rather than a raw
+     * microcode container.  Look within it.
+     */
     if ( opt_scan )
-        /* Need to rescan the modules because they might have been relocated */
-        microcode_scan_module(bi);
+    {
+        struct cpio_data cd = find_cpio_data(ucode_ops.cpio_path, data, size);
 
-    if ( ucode_mod.mod_end )
-        rc = early_update_cache(bootstrap_map(&ucode_mod),
-                                ucode_mod.mod_end);
-    else if ( ucode_blob.size )
-        rc = early_update_cache(ucode_blob.data, ucode_blob.size);
+        if ( !cd.data )
+        {
+            printk(XENLOG_WARNING "Microcode: %s not found in CPIO archive\n",
+                   strrchr(ucode_ops.cpio_path, '/') + 1);
+            return -ENOENT;
+        }
+
+        data = cd.data;
+        size = cd.size;
+    }
+
+    rc = early_update_cache(data, size);
 
     return rc;
 }
@@ -819,7 +788,7 @@ presmp_initcall(microcode_init_cache);
  */
 static int __init early_microcode_load(struct boot_info *bi)
 {
-    const void *data = NULL;
+    void *data = NULL;
     size_t size;
     struct microcode_patch *patch;
     int idx = opt_mod_idx;
@@ -832,7 +801,40 @@ static int __init early_microcode_load(struct boot_info 
*bi)
     ASSERT(idx == 0 || !opt_scan);
 
     if ( opt_scan ) /* Scan for a CPIO archive */
-        microcode_scan_module(bi);
+    {
+        for ( idx = 1; idx < bi->nr_modules; ++idx )
+        {
+            struct cpio_data cd;
+
+            if ( !test_bit(idx, bi->module_map) )
+                continue;
+
+            size = bi->mods[idx].mod->mod_end;
+            data = bootstrap_map_bm(&bi->mods[idx]);
+            if ( !data )
+            {
+                printk(XENLOG_WARNING "Microcode: Could not map module %d, 
size %zu\n",
+                       idx, size);
+                continue;
+            }
+
+            cd = find_cpio_data(ucode_ops.cpio_path, data, size);
+            if ( !cd.data )
+            {
+                /* CPIO archive, but no cpio_path.  Try the next module */
+                bootstrap_unmap();
+                continue;
+            }
+
+            data = cd.data;
+            size = cd.size;
+            goto found;
+        }
+
+        printk(XENLOG_WARNING "Microcode: %s not found during CPIO scan\n",
+               strrchr(ucode_ops.cpio_path, '/') + 1);
+        return -ENODEV;
+    }
 
     if ( idx ) /* Specific module nominated */
     {
@@ -856,26 +858,21 @@ static int __init early_microcode_load(struct boot_info 
*bi)
             return -ENODEV;
         }
 
-        ucode_mod = *bi->mods[idx].mod;
-    }
-
-    if ( !ucode_mod.mod_end && !ucode_blob.size )
-        return 0;
-
-    if ( ucode_blob.size )
-    {
-        size = ucode_blob.size;
-        data = ucode_blob.data;
-    }
-    else if ( ucode_mod.mod_end )
-    {
-        size = ucode_mod.mod_end;
-        data = bootstrap_map(&ucode_mod);
+        size = bi->mods[idx].mod->mod_end;
+        data = bootstrap_map_bm(&bi->mods[idx]);
+        if ( !data )
+        {
+            printk(XENLOG_WARNING "Microcode: Could not map module %d, size 
%zu\n",
+                   idx, size);
+            return -ENODEV;
+        }
+        goto found;
     }
 
-    if ( !data )
-        return -ENOMEM;
+    /* No method of finding microcode specified.  Nothing to do. */
+    return 0;
 
+ found:
     patch = ucode_ops.cpu_request_microcode(data, size, false);
     if ( IS_ERR(patch) )
     {
@@ -891,6 +888,15 @@ static int __init early_microcode_load(struct boot_info 
*bi)
         goto unmap;
     }
 
+    /*
+     * We've found a microcode patch suitable for this CPU.
+     *
+     * Tell microcode_init_cache() which module we found it in.  We cache it
+     * irrespective of whether the BSP successfully loads it; Some platforms
+     * are known to update the BSP but leave the APs on older ucode.
+     */
+    early_mod_idx = idx;
+
     rc = microcode_update_cpu(patch, 0);
 
  unmap:
-- 
2.39.5




 


Rackspace

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