2018-03-15 23:11:06

by Maciej S. Szmigiero

[permalink] [raw]
Subject: [PATCH v4 02/10] x86/microcode/AMD: Check equivalence table length in the early loader

Before loading a CPU equivalence table from a microcode container file we
need to verify whether this file is actually large enough to contain the
table of a size indicated in this file.
If it is not, there is no point of continuing with loading it since
microcode patches are located after the equivalence table anyway.

This patch adds these checks to the early loader.

Signed-off-by: Maciej S. Szmigiero <[email protected]>
---
arch/x86/kernel/cpu/microcode/amd.c | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 6a93be0f771c..138c9fb983f2 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -80,20 +80,33 @@ 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;
+ unsigned int cont_magic, cont_type;
+ size_t equiv_tbl_len;
u16 eq_id;
u8 *buf;

+ if (size < CONTAINER_HDR_SZ)
+ return 0;
+
+ cont_magic = hdr[0];
+ cont_type = hdr[1];
+ equiv_tbl_len = hdr[2];
+
/* Am I looking at an equivalence table header? */
- if (hdr[0] != UCODE_MAGIC ||
- hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
- hdr[2] == 0)
+ if (cont_magic != UCODE_MAGIC ||
+ cont_type != UCODE_EQUIV_CPU_TABLE_TYPE ||
+ equiv_tbl_len == 0)
return CONTAINER_HDR_SZ;

+ if (equiv_tbl_len < sizeof(*eq) ||
+ size - CONTAINER_HDR_SZ < equiv_tbl_len)
+ return size;
+
buf = ucode;

eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
@@ -101,8 +114,8 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
/* Find the equivalence ID of our CPU in this table: */
eq_id = find_equiv_id(eq, desc->cpuid_1_eax);

- buf += hdr[2] + CONTAINER_HDR_SZ;
- size -= hdr[2] + CONTAINER_HDR_SZ;
+ buf += equiv_tbl_len + CONTAINER_HDR_SZ;
+ size -= equiv_tbl_len + CONTAINER_HDR_SZ;

/*
* Scan through the rest of the container to find where it ends. We do
@@ -159,15 +172,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;
}
}



2018-03-20 15:45:10

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v4 02/10] x86/microcode/AMD: Check equivalence table length in the early loader

On Fri, Mar 16, 2018 at 12:07:50AM +0100, Maciej S. Szmigiero wrote:
> Before loading a CPU equivalence table from a microcode container file we
> need to verify whether this file is actually large enough to contain the
> table of a size indicated in this file.
> If it is not, there is no point of continuing with loading it since
> microcode patches are located after the equivalence table anyway.
>
> This patch adds these checks to the early loader.
>
> Signed-off-by: Maciej S. Szmigiero <[email protected]>
> ---
> arch/x86/kernel/cpu/microcode/amd.c | 35 +++++++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
> index 6a93be0f771c..138c9fb983f2 100644
> --- a/arch/x86/kernel/cpu/microcode/amd.c
> +++ b/arch/x86/kernel/cpu/microcode/amd.c
> @@ -80,20 +80,33 @@ 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;
> + unsigned int cont_magic, cont_type;
> + size_t equiv_tbl_len;
> u16 eq_id;
> u8 *buf;
>
> + if (size < CONTAINER_HDR_SZ)
> + return 0;
> +
> + cont_magic = hdr[0];
> + cont_type = hdr[1];
> + equiv_tbl_len = hdr[2];

All three are u32.

> /* Am I looking at an equivalence table header? */
> - if (hdr[0] != UCODE_MAGIC ||
> - hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
> - hdr[2] == 0)
> + if (cont_magic != UCODE_MAGIC ||
> + cont_type != UCODE_EQUIV_CPU_TABLE_TYPE ||
> + equiv_tbl_len == 0)
> return CONTAINER_HDR_SZ;
>
> + if (equiv_tbl_len < sizeof(*eq) ||

If you do this, then the above == 0 check can go.

> + size - CONTAINER_HDR_SZ < equiv_tbl_len)
> + return size;
> +
> buf = ucode;
>
> eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply. Srsly.