[xen master] x86/ucode/amd: Handle length sanity check failures more gracefully

[patchbot@xxxxxxx]

commit 0e898ad8bc86d76485ce7a6a29ff2d3fa34d070d
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Tue Feb 9 20:49:07 2021 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Wed Feb 10 13:23:51 2021 +0000

    x86/ucode/amd: Handle length sanity check failures more gracefully
    
    Currently, a failure of verify_patch_size() causes an early abort of the
    microcode blob loop, which in turn causes a second go around the main
    container loop, ultimately failing the UCODE_MAGIC check.
    
    First, check for errors after the blob loop.  An error here is 
unrecoverable,
    so avoid going around the container loop again and printing an
    unhelpful-at-best error concerning bad UCODE_MAGIC.
    
    Second, split the verify_patch_size() check out of the microcode blob header
    check.  In the case that the sanity check fails, we can still use the
    known-to-be-plausible header length to continue walking the container to
    potentially find other applicable microcode blobs.
    
    Before:
      (XEN) microcode: Bad microcode data
      (XEN) microcode: Wrong microcode patch file magic
      (XEN) Parsing microcode blob error -22
    
    After:
      (XEN) microcode: Bad microcode length 0x000015c0 for cpu 0xa000
      (XEN) microcode: Bad microcode length 0x000015c0 for cpu 0xa010
      (XEN) microcode: Bad microcode length 0x000015c0 for cpu 0xa011
      (XEN) microcode: Bad microcode length 0x000015c0 for cpu 0xa200
      (XEN) microcode: Bad microcode length 0x000015c0 for cpu 0xa210
      (XEN) microcode: Bad microcode length 0x000015c0 for cpu 0xa500
      (XEN) microcode: couldn't find any matching ucode in the provided blob!
    
    Fixes: 4de936a38a ("x86/ucode/amd: Rework parsing logic in 
cpu_request_microcode()")
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/cpu/microcode/amd.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 3e51e53c1f..500b2ce941 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -349,8 +349,7 @@ static struct microcode_patch *cpu_request_microcode(const 
void *buf, size_t siz
             if ( size < sizeof(*mc) ||
                  (mc = buf)->type != UCODE_UCODE_TYPE ||
                  size - sizeof(*mc) < mc->len ||
-                 mc->len < sizeof(struct microcode_patch) ||
-                 (!skip_ucode && !verify_patch_size(mc->len)) )
+                 mc->len < sizeof(struct microcode_patch) )
             {
                 printk(XENLOG_ERR "microcode: Bad microcode data\n");
                 error = -EINVAL;
@@ -360,6 +359,19 @@ static struct microcode_patch *cpu_request_microcode(const 
void *buf, size_t siz
             if ( skip_ucode )
                 goto skip;
 
+            if ( !verify_patch_size(mc->len) )
+            {
+                printk(XENLOG_WARNING
+                       "microcode: Bad microcode length 0x%08x for cpu 
0x%04x\n",
+                       mc->len, mc->patch->processor_rev_id);
+                /*
+                 * If the blob size sanity check fails, trust the container
+                 * length which has already been checked to be at least
+                 * plausible at this point.
+                 */
+                goto skip;
+            }
+
             /*
              * If the new ucode covers current CPU, compare ucodes and store 
the
              * one with higher revision.
@@ -383,6 +395,14 @@ static struct microcode_patch *cpu_request_microcode(const 
void *buf, size_t siz
             if ( size >= 4 && *(const uint32_t *)buf == UCODE_MAGIC )
                 break;
         }
+
+        /*
+         * Any error means we didn't get cleanly to the end of the microcode
+         * container.  There isn't an overall length field, so we've got no
+         * way of skipping to the next container in the stream.
+         */
+        if ( error )
+            break;
     }
 
     if ( saved )
--
generated by git-patchbot for /home/xen/git/xen.git#master