summaryrefslogtreecommitdiff
path: root/src/cpu/intel
diff options
context:
space:
mode:
authorVladimir Serbinenko <phcoder@gmail.com>2013-03-30 12:04:23 +0100
committerRonald G. Minnich <rminnich@gmail.com>2013-04-23 03:30:22 +0200
commit2c88cc0696be2b01ebd4df6f7593d8658c8fe419 (patch)
tree0599c602c43cc928b65e2e96835e4e29567f6df9 /src/cpu/intel
parentaee444f453f38e53f3e2ac54b560707616767869 (diff)
Intel microcode: Return when `microcode_updates` is `NULL`
Add a safety check in function `intel_update_microcode` to return when accidentally `NULL` is passed as `microcode_updates`, which would lead to a null pointer dereference later on. for (c = microcode_updates; m->hdrver; m = (const struct microcode *)c) { While at it, use `return NULL` for clarity in function `intel_microcode_find` and include the header file `stddef.h`. for it. The review of this patch had some more discussion on adding more comments and more detailed error messages. But this should be done in a separate patch. For clarity here some history, on how this was found and what caused the discussion and confusion. Originally when Vladimir made this improvement, selecting `CPU_MICROCODE_IN_CBFS` in Kconfig but not having the microcode blob `cpu_microcode_blob.bin` in CBFS resulted in a null pointer dereference later on causing a crash. for (c = microcode_updates; m->hdrver; m = (const struct microcode *)c) { Vladimir fixed this by returning if `microcode_updates` is `NULL`, that means no file is found and successfully tested this on his Lenovo X201. When pushing the patch to Gerrit for review, the code was rewritten though by Aaron in commit »intel microcode: split up microcode loading stages« (98ffb426) [1], which also returns when no file is found. So the other parts of the code were checked and the safety check as described above is added. [1] http://review.coreboot.org/2778 Change-Id: I6e18fd37256910bf047061e4633a66cf29ad7b69 Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com> Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-on: http://review.coreboot.org/2990 Reviewed-by: Aaron Durbin <adurbin@google.com> Tested-by: build bot (Jenkins)
Diffstat (limited to 'src/cpu/intel')
-rw-r--r--src/cpu/intel/microcode/microcode.c10
1 files changed, 9 insertions, 1 deletions
diff --git a/src/cpu/intel/microcode/microcode.c b/src/cpu/intel/microcode/microcode.c
index d908c25ec0..1991ed8472 100644
--- a/src/cpu/intel/microcode/microcode.c
+++ b/src/cpu/intel/microcode/microcode.c
@@ -21,6 +21,7 @@
/* Microcode update for Intel PIII and later CPUs */
#include <stdint.h>
+#include <stddef.h>
#if !defined(__ROMCC__)
#include <console/console.h>
#endif
@@ -131,7 +132,7 @@ const void *intel_microcode_find(void)
#endif
if (!microcode_updates)
- return microcode_updates;
+ return NULL;
/* CPUID sets MSR 0x8B iff a microcode update has been loaded. */
msr.lo = 0;
@@ -202,6 +203,13 @@ void intel_update_microcode(const void *microcode_updates)
const char *c;
msr_t msr;
+ if (!microcode_updates) {
+#if !defined(__ROMCC__)
+ printk(BIOS_WARNING, "No microcode updates found.\n");
+#endif
+ return;
+ }
+
/* CPUID sets MSR 0x8B iff a microcode update has been loaded. */
msr.lo = 0;
msr.hi = 0;