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.coop...@citrix.com>
---
CC: Jan Beulich <jbeul...@suse.com>
CC: Roger Pau Monné <roger....@citrix.com>
CC: Wei Liu <w...@xen.org>
CC: Ian Jackson <i...@xenproject.org>
For 4.15. Found when putting a test together to prove the correctness of the
"x86/ucode: Fix microcode payload size for Fam19 processors" fix.
This allows microcode loading to still function even if the length magic
numbers aren't correct for a subset of blobs within the container(s).
---
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 c4ab395799..fe7b79bd0a 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -348,8 +348,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 ||
- (!skip_ucode && !verify_patch_size(mc->len)) )
+ size - sizeof(*mc) < mc->len )
{
printk(XENLOG_ERR "microcode: Bad microcode data\n");
error = -EINVAL;
@@ -359,6 +358,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.
@@ -382,6 +394,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 )
--
2.11.0
