Currently, it is very easy to make the AMD microcode update driver crash or spin on a malformed microcode file since it does very little consistency checking on data loaded from such file.
This commit introduces various checks, mostly on length-type fields, so all corrupted microcode files are (hopefully) correctly rejected instead. Signed-off-by: Maciej S. Szmigiero <m...@maciej.szmigiero.name> --- Test files are at https://pastebin.com/XkKUSmMp One has to enable KASAN in the kernel config and rename a particular test file to name appropriate to the running CPU family to test its loading. arch/x86/include/asm/microcode_amd.h | 6 ++ arch/x86/kernel/cpu/microcode/amd.c | 111 ++++++++++++++++++++++++++--------- 2 files changed, 89 insertions(+), 28 deletions(-) diff --git a/arch/x86/include/asm/microcode_amd.h b/arch/x86/include/asm/microcode_amd.h index 209492849566..e797d5d939d7 100644 --- a/arch/x86/include/asm/microcode_amd.h +++ b/arch/x86/include/asm/microcode_amd.h @@ -19,6 +19,9 @@ struct equiv_cpu_entry { u16 res; } __attribute__((packed)); +/* 4k */ +#define UCODE_EQUIV_CPU_TABLE_MAX_SIZE (256 * sizeof(struct equiv_cpu_entry)) + struct microcode_header_amd { u32 data_code; u32 patch_id; @@ -41,6 +44,9 @@ struct microcode_amd { unsigned int mpb[0]; }; +/* if a patch is larger than this the microcode file is surely corrupted */ +#define PATCH_MAX_SIZE_ABSOLUTE (16 * 1024 * 1024) + #define PATCH_MAX_SIZE PAGE_SIZE #ifdef CONFIG_MICROCODE_AMD diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c index a998e1a7d46f..f798bd4004aa 100644 --- a/arch/x86/kernel/cpu/microcode/amd.c +++ b/arch/x86/kernel/cpu/microcode/amd.c @@ -39,6 +39,7 @@ #include <asm/msr.h> static struct equiv_cpu_entry *equiv_cpu_table; +static unsigned int equiv_cpu_table_size; /* * This points to the current valid container of microcode patches which we will @@ -63,12 +64,17 @@ static u8 amd_ucode_patch[PATCH_MAX_SIZE]; static const char ucode_path[] __maybe_unused = "kernel/x86/microcode/AuthenticAMD.bin"; -static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig) +static u16 find_equiv_id(const struct equiv_cpu_entry *equiv_table, + unsigned int equiv_table_size, u32 sig) { - for (; equiv_table && equiv_table->installed_cpu; equiv_table++) { - if (sig == equiv_table->installed_cpu) - return equiv_table->equiv_cpu; - } + unsigned int i; + + if (!equiv_table) + return 0; + + for (i = 0; i < equiv_table_size && equiv_table[i].installed_cpu; i++) + if (sig == equiv_table[i].installed_cpu) + return equiv_table[i].equiv_cpu; return 0; } @@ -80,29 +86,41 @@ static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig) * Returns the amount of bytes consumed while scanning. @desc contains all the * data we're going to use in later stages of the application. */ -static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc) +static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc) { struct equiv_cpu_entry *eq; - ssize_t orig_size = size; + size_t orig_size = size; u32 *hdr = (u32 *)ucode; + u32 eqsize; u16 eq_id; u8 *buf; + if (size < CONTAINER_HDR_SZ) + return 0; + /* Am I looking at an equivalence table header? */ if (hdr[0] != UCODE_MAGIC || hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE || hdr[2] == 0) return CONTAINER_HDR_SZ; + eqsize = hdr[2]; + if (eqsize < sizeof(*eq) || + eqsize > UCODE_EQUIV_CPU_TABLE_MAX_SIZE) + return 0; + + if (size < CONTAINER_HDR_SZ + eqsize) + return 0; + buf = ucode; eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ); /* Find the equivalence ID of our CPU in this table: */ - eq_id = find_equiv_id(eq, desc->cpuid_1_eax); + eq_id = find_equiv_id(eq, eqsize / sizeof(*eq), desc->cpuid_1_eax); - buf += hdr[2] + CONTAINER_HDR_SZ; - size -= hdr[2] + CONTAINER_HDR_SZ; + buf += eqsize + CONTAINER_HDR_SZ; + size -= eqsize + CONTAINER_HDR_SZ; /* * Scan through the rest of the container to find where it ends. We do @@ -112,6 +130,9 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc) struct microcode_amd *mc; u32 patch_size; + if (size < SECTION_HDR_SIZE) + break; + hdr = (u32 *)buf; if (hdr[0] != UCODE_UCODE_TYPE) @@ -126,6 +147,10 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc) buf += SECTION_HDR_SIZE; size -= SECTION_HDR_SIZE; + if (size < sizeof(*mc) || + size < patch_size) + break; + mc = (struct microcode_amd *)buf; if (eq_id == mc->hdr.processor_rev_id) { desc->psize = patch_size; @@ -159,15 +184,13 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc) */ static void scan_containers(u8 *ucode, size_t size, struct cont_desc *desc) { - ssize_t rem = size; - - while (rem >= 0) { - ssize_t s = parse_container(ucode, rem, desc); + while (size > 0) { + size_t s = parse_container(ucode, size, desc); if (!s) return; ucode += s; - rem -= s; + size -= s; } } @@ -364,20 +387,21 @@ void reload_ucode_amd(void) static u16 __find_equiv_id(unsigned int cpu) { struct ucode_cpu_info *uci = ucode_cpu_info + cpu; - return find_equiv_id(equiv_cpu_table, uci->cpu_sig.sig); + return find_equiv_id(equiv_cpu_table, equiv_cpu_table_size, + uci->cpu_sig.sig); } static u32 find_cpu_family_by_equiv_cpu(u16 equiv_cpu) { - int i = 0; + unsigned int i; BUG_ON(!equiv_cpu_table); - while (equiv_cpu_table[i].equiv_cpu != 0) { + for (i = 0; i < equiv_cpu_table_size && + equiv_cpu_table[i].equiv_cpu != 0; i++) if (equiv_cpu == equiv_cpu_table[i].equiv_cpu) return equiv_cpu_table[i].installed_cpu; - i++; - } + return 0; } @@ -540,15 +564,31 @@ static enum ucode_state apply_microcode_amd(int cpu) return UCODE_UPDATED; } -static int install_equiv_cpu_table(const u8 *buf) +static int install_equiv_cpu_table(const u8 *buf, size_t bufsize) { unsigned int *ibuf = (unsigned int *)buf; - unsigned int type = ibuf[1]; - unsigned int size = ibuf[2]; + unsigned int type, size; - if (type != UCODE_EQUIV_CPU_TABLE_TYPE || !size) { - pr_err("empty section/" - "invalid type field in container file section header\n"); + if (bufsize < CONTAINER_HDR_SZ) { + pr_err("no container header\n"); + return -EINVAL; + } + + type = ibuf[1]; + if (type != UCODE_EQUIV_CPU_TABLE_TYPE) { + pr_err("invalid type field in container file section header\n"); + return -EINVAL; + } + + size = ibuf[2]; + if (size < sizeof(struct equiv_cpu_entry) || + size > UCODE_EQUIV_CPU_TABLE_MAX_SIZE) { + pr_err("equivalent CPU table size invalid\n"); + return -EINVAL; + } + + if (bufsize < CONTAINER_HDR_SZ + size) { + pr_err("equivalent CPU table truncated\n"); return -EINVAL; } @@ -559,6 +599,7 @@ static int install_equiv_cpu_table(const u8 *buf) } memcpy(equiv_cpu_table, buf + CONTAINER_HDR_SZ, size); + equiv_cpu_table_size = size / sizeof(struct equiv_cpu_entry); /* add header length */ return size + CONTAINER_HDR_SZ; @@ -568,6 +609,7 @@ static void free_equiv_cpu_table(void) { vfree(equiv_cpu_table); equiv_cpu_table = NULL; + equiv_cpu_table_size = 0; } static void cleanup(void) @@ -591,7 +633,15 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover) u32 proc_fam; u16 proc_id; + if (leftover < SECTION_HDR_SIZE + sizeof(*mc_hdr)) + return leftover; + patch_size = *(u32 *)(fw + 4); + if (patch_size > PATCH_MAX_SIZE_ABSOLUTE) { + pr_err("mammoth patch size %u\n", patch_size); + return -EINVAL; + } + crnt_size = patch_size + SECTION_HDR_SIZE; mc_hdr = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE); proc_id = mc_hdr->processor_rev_id; @@ -613,7 +663,8 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover) return crnt_size; } - ret = verify_patch_size(family, patch_size, leftover); + ret = verify_patch_size(family, patch_size, + leftover - SECTION_HDR_SIZE); if (!ret) { pr_err("Patch-ID 0x%08x: size mismatch.\n", mc_hdr->patch_id); return crnt_size; @@ -654,7 +705,7 @@ static enum ucode_state __load_microcode_amd(u8 family, const u8 *data, int crnt_size = 0; int offset; - offset = install_equiv_cpu_table(data); + offset = install_equiv_cpu_table(data, size); if (offset < 0) { pr_err("failed to create equivalent cpu table\n"); return ret; @@ -745,6 +796,10 @@ static enum ucode_state request_microcode_amd(int cpu, struct device *device, } ret = UCODE_ERROR; + if (fw->size < sizeof(u32)) { + pr_err("microcode far too short\n"); + goto fw_release; + } if (*(u32 *)fw->data != UCODE_MAGIC) { pr_err("invalid magic value (0x%08x)\n", *(u32 *)fw->data); goto fw_release;
