[Xen-changelog] [xen staging] x86/ucode/intel: Remove gratuitous memory allocations from cpu_request_microcode()

[patchbot@xxxxxxx]

commit b362416637d1121e4c0bd25c48becb8b73765844
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Fri Mar 20 17:01:33 2020 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Wed Apr 1 14:00:12 2020 +0100

    x86/ucode/intel: Remove gratuitous memory allocations from 
cpu_request_microcode()
    
    cpu_request_microcode() needs to scan its container and duplicate one blob,
    but the get_next_ucode_from_buffer() helper duplicates every blob in turn.
    Furthermore, the length checking is only safe from overflow in 64bit builds.
    
    Delete get_next_ucode_from_buffer() and alter the purpose of the saved
    variable to simply point somewhere in buf until we're ready to return.
    
    This is only a modest reduction in absolute code size, but avoids making
    memory allocations for every blob in the container.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/cpu/microcode/intel.c | 65 +++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 40 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index 60b08d2446..9a1170a870 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -311,67 +311,52 @@ static int apply_microcode(const struct microcode_patch 
*patch)
     return 0;
 }
 
-static long get_next_ucode_from_buffer(struct microcode_intel **mc,
-                                       const uint8_t *buf, unsigned long size,
-                                       unsigned long offset)
-{
-    struct microcode_header_intel *mc_header;
-    unsigned long total_size;
-
-    /* No more data */
-    if ( offset >= size )
-        return 0;
-    mc_header = (struct microcode_header_intel *)(buf + offset);
-    total_size = get_totalsize(mc_header);
-
-    if ( (offset + total_size) > size )
-    {
-        printk(KERN_ERR "microcode: error! Bad data in microcode data file\n");
-        return -EINVAL;
-    }
-
-    *mc = xmemdup_bytes(mc_header, total_size);
-    if ( *mc == NULL )
-        return -ENOMEM;
-
-    return offset + total_size;
-}
-
 static struct microcode_patch *cpu_request_microcode(const void *buf,
                                                      size_t size)
 {
-    long offset = 0;
     int error = 0;
-    struct microcode_intel *mc, *saved = NULL;
+    const struct microcode_patch *saved = NULL;
     struct microcode_patch *patch = NULL;
 
-    while ( (offset = get_next_ucode_from_buffer(&mc, buf, size, offset)) > 0 )
+    while ( size )
     {
-        error = microcode_sanity_check(mc);
-        if ( error )
+        const struct microcode_patch *mc;
+        unsigned int blob_size;
+
+        if ( size < MC_HEADER_SIZE ||       /* Insufficient space for header? 
*/
+             (mc = buf)->hdr.hdrver != 1 || /* Unrecognised header version?   
*/
+             mc->hdr.ldrver != 1 ||         /* Unrecognised loader version?   
*/
+             size < (blob_size =            /* Insufficient space for patch?  
*/
+                     get_totalsize(&mc->hdr)) )
         {
-            xfree(mc);
+            error = -EINVAL;
+            printk(XENLOG_WARNING "microcode: Bad data in container\n");
             break;
         }
 
+        error = microcode_sanity_check(mc);
+        if ( error )
+            break;
+
         /*
          * If the new update covers current CPU, compare updates and store the
          * one with higher revision.
          */
         if ( (microcode_update_match(mc) != MIS_UCODE) &&
              (!saved || (mc->hdr.rev > saved->hdr.rev)) )
-        {
-            xfree(saved);
             saved = mc;
-        }
-        else
-            xfree(mc);
+
+        buf  += blob_size;
+        size -= blob_size;
     }
-    if ( offset < 0 )
-        error = offset;
 
     if ( saved )
-        patch = saved;
+    {
+        patch = xmemdup_bytes(saved, get_totalsize(&saved->hdr));
+
+        if ( !patch )
+            error = -ENOMEM;
+    }
 
     if ( error && !patch )
         patch = ERR_PTR(error);
--
generated by git-patchbot for /home/xen/git/xen.git#staging

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog