[06/11] x86/ucode/amd: Move verify_patch_size() into get_ucode_from_buffer_amd()

Commit Message

Andrew Cooper March 31, 2020, 10:05 a.m. UTC

We only stash the microcode blob size so it can be audited in
microcode_fits().  However, the patch size check depends only on the CPU
family.

Move the check earlier to when we are parsing the container, which avoids
caching bad microcode in the first place, and allows us to avoid storing the
size at all.

Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: Jan Beulich <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Roger Pau Monné <[e-mail]>
---
 xen/arch/x86/cpu/microcode/amd.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

Comments

Jan Beulich March 31, 2020, 2:38 p.m. UTC | #1

On 31.03.2020 12:05, Andrew Cooper wrote:
> We only stash the microcode blob size so it can be audited in
> microcode_fits().  However, the patch size check depends only on the CPU
> family.
> 
> Move the check earlier to when we are parsing the container, which avoids
> caching bad microcode in the first place, and allows us to avoid storing the
> size at all.
> 
> Signed-off-by: Andrew Cooper <[e-mail]>

Reviewed-by: Jan Beulich <[e-mail]>

Patch

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index d3439b0c6c..8318664f85 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -60,7 +60,6 @@  struct __packed microcode_header_amd {
 
 struct microcode_patch {
     struct microcode_header_amd *mpb;
-    size_t mpb_size;
 };
 
 /* Temporary, until the microcode_* structure are disentangled. */
@@ -184,12 +183,6 @@  static enum microcode_match_result microcode_fits(
          equiv.id  != mc_header->processor_rev_id )
         return MIS_UCODE;
 
-    if ( !verify_patch_size(mc_amd->mpb_size) )
-    {
-        pr_debug("microcode: patch size mismatch\n");
-        return MIS_UCODE;
-    }
-
     if ( mc_header->patch_id <= sig->rev )
     {
         pr_debug("microcode: patch is already at required level or greater.\n");
@@ -318,10 +311,15 @@  static int get_ucode_from_buffer_amd(
         return -EINVAL;
     }
 
+    if ( !verify_patch_size(mpbuf->len) )
+    {
+        printk(XENLOG_ERR "microcode: patch size mismatch\n");
+        return -EINVAL;
+    }
+
     mc_amd->mpb = xmemdup_bytes(mpbuf->data, mpbuf->len);
     if ( !mc_amd->mpb )
         return -ENOMEM;
-    mc_amd->mpb_size = mpbuf->len;
 
     pr_debug("microcode: CPU%d size %zu, block size %u offset %zu equivID %#x rev %#x\n",
              smp_processor_id(), bufsize, mpbuf->len, *offset,
@@ -439,7 +437,7 @@  static struct microcode_patch *cpu_request_microcode(const void *buf,
     struct microcode_amd *mc_amd;
     struct microcode_header_amd *saved = NULL;
     struct microcode_patch *patch = NULL;
-    size_t offset = 0, saved_size = 0;
+    size_t offset = 0;
     int error = 0;
     unsigned int cpu = smp_processor_id();
     const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
@@ -516,7 +514,6 @@  static struct microcode_patch *cpu_request_microcode(const void *buf,
         {
             xfree(saved);
             saved = mc_amd->mpb;
-            saved_size = mc_amd->mpb_size;
         }
         else
         {
@@ -555,7 +552,6 @@  static struct microcode_patch *cpu_request_microcode(const void *buf,
     if ( saved )
     {
         mc_amd->mpb = saved;
-        mc_amd->mpb_size = saved_size;
         patch = mc_amd;
     }
     else