[PATCH 00/16] x86/microcode/AMD: Improve container verification

[PATCH 00/16] x86/microcode/AMD: Improve container verification

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Hi all,

this is work which got started by Maciej a while ago. I have finally had
the time to redo all the ideas properly, split it in self-contained,
logical chunks and test it.

Reveiew and comments are appreciated.

Thx.

Borislav Petkov (11):
  x86/microcode/AMD: Move verify_patch_size() up in the file
  x86/microcode/AMD: Clean up per-family patch size checks
  x86/microcode/AMD: Cleanup verify_patch_size() more
  x86/microcode/AMD: Concentrate patch verification
  x86/microcode/AMD: Simplify patch family detection
  x86/microcode/AMD: Move patch family check to verify_patch()
  x86/microcode/AMD: Move chipset-specific check into verify_patch()
  x86/microcode/AMD: Change verify_patch()'s return value
  x86/microcode/AMD: Convert early parser to the new verification routines
  x86/microcode/AMD: Fix container size's type
  x86/microcode/AMD: Update copyright

Maciej S. Szmigiero (5):
  x86/microcode/AMD: Subtract SECTION_HDR_SIZE from file leftover length
  x86/microcode/AMD: Add microcode container verification
  x86/microcode/AMD: Check microcode container data in the late loader
  x86/microcode/AMD: Convert CPU equivalence table variable into a struct
  x86/microcode/AMD: Check the equivalence table size when scanning it

 arch/x86/kernel/cpu/microcode/amd.c | 469 ++++++++++++++++++----------
 1 file changed, 307 insertions(+), 162 deletions(-)

-- 
2.19.1

[PATCH 01/16] x86/microcode/AMD: Subtract SECTION_HDR_SIZE from file leftover length

[Borislav Petkov]

From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>

verify_patch_size() verifies whether the remaining size of the microcode
container file is large enough to contain a patch of the indicated size.

However, the section header length is not included in this indicated
size but it is present in the leftover file length so it should be
subtracted from the leftover file length before passing this value to
verify_patch_size().

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Cc: x86-ml <x86@kernel.org>
Link: http://lkml.kernel.org/r/6df43f4f6a28186a13a66e8d7e61143c5e1a2324.1529424596.git.mail@maciej.szmigiero.name
[ Split comment. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 07b5fc00b188..b44c9029860c 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -461,8 +461,12 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig)
 	return 0;
 }
 
-static unsigned int verify_patch_size(u8 family, u32 patch_size,
-				      unsigned int size)
+/*
+ * Check whether the passed remaining file @size is large enough to contain a
+ * patch of the indicated @patch_size (and also whether this size does not
+ * exceed the per-family maximum).
+ */
+static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int size)
 {
 	u32 max_size;
 
@@ -621,7 +625,12 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 		return crnt_size;
 	}
 
-	ret = verify_patch_size(family, patch_size, leftover);
+	/*
+	 * The section header length is not included in this indicated size
+	 * but is present in the leftover file length so we need to subtract
+	 * it before passing this value to the function below.
+	 */
+	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;
-- 
2.19.1

[PATCH 02/16] x86/microcode/AMD: Add microcode container verification

[Borislav Petkov]

From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>

Add container and patch verification functions to the AMD microcode
update driver.

These functions check whether a passed buffer contains the relevant
structure, whether it isn't truncated and (for actual microcode patches)
whether the size of a patch is not too large for a particular CPU family.
By adding these checks as separate functions the actual microcode loading
code won't get interspersed with a lot of checks and so will be more
readable.

 [ bp: Make all pr_err() calls into pr_debug(). ]

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Cc: x86-ml <x86@kernel.org>
Link: http://lkml.kernel.org/r/3014e96c82cd90761b4601bd2cfe59c4119e46a7.1529424596.git.mail@maciej.szmigiero.name
[ Drop the verify_patch() bits. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 101 ++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index b44c9029860c..36aeb01b3ae8 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -73,6 +73,107 @@ static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig)
 	return 0;
 }
 
+/*
+ * Check whether there is a valid microcode container file at the beginning
+ * of @buf of size @buf_size. Set @early to use this function in the early path.
+ */
+static bool verify_container(const u8 *buf, size_t buf_size, bool early)
+{
+	u32 cont_magic;
+
+	if (buf_size <= CONTAINER_HDR_SZ) {
+		if (!early)
+			pr_debug("Truncated microcode container header.\n");
+
+		return false;
+	}
+
+	cont_magic = *(const u32 *)buf;
+	if (cont_magic != UCODE_MAGIC) {
+		if (!early)
+			pr_debug("Invalid magic value (0x%08x).\n", cont_magic);
+
+		return false;
+	}
+
+	return true;
+}
+
+/*
+ * Check whether there is a valid, non-truncated CPU equivalence table at the
+ * beginning of @buf of size @buf_size. Set @early to use this function in the
+ * early path.
+ */
+static bool verify_equivalence_table(const u8 *buf, size_t buf_size, bool early)
+{
+	const u32 *hdr = (const u32 *)buf;
+	u32 cont_type, equiv_tbl_len;
+
+	if (!verify_container(buf, buf_size, early))
+		return false;
+
+	cont_type = hdr[1];
+	if (cont_type != UCODE_EQUIV_CPU_TABLE_TYPE) {
+		if (!early)
+			pr_debug("Wrong microcode container equivalence table type: %u.\n",
+			       cont_type);
+
+		return false;
+	}
+
+	buf_size -= CONTAINER_HDR_SZ;
+
+	equiv_tbl_len = hdr[2];
+	if (equiv_tbl_len < sizeof(struct equiv_cpu_entry) ||
+	    buf_size < equiv_tbl_len) {
+		if (!early)
+			pr_debug("Truncated equivalence table.\n");
+
+		return false;
+	}
+
+	return true;
+}
+
+/*
+ * Check whether there is a valid, non-truncated microcode patch section at the
+ * beginning of @buf of size @buf_size. Set @early to use this function in the
+ * early path.
+ */
+static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
+{
+	const u32 *hdr;
+	u32 patch_type, patch_size;
+
+	if (buf_size < SECTION_HDR_SIZE) {
+		if (!early)
+			pr_debug("Truncated patch section.\n");
+
+		return false;
+	}
+
+	hdr = (const u32 *)buf;
+	patch_type = hdr[0];
+	patch_size = hdr[1];
+
+	if (patch_type != UCODE_UCODE_TYPE) {
+		if (!early)
+			pr_debug("Invalid type field (0x%x) in container file section header.\n",
+				patch_type);
+
+		return false;
+	}
+
+	if (patch_size < sizeof(struct microcode_header_amd)) {
+		if (!early)
+			pr_debug("Patch of size %u too short.\n", patch_size);
+
+		return false;
+	}
+
+	return true;
+}
+
 /*
  * This scans the ucode blob for the proper container as we can have multiple
  * containers glued together. Returns the equivalence ID from the equivalence
-- 
2.19.1

[PATCH 03/16] x86/microcode/AMD: Move verify_patch_size() up in the file

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

... to enable later improvements.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 82 ++++++++++++++---------------
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 36aeb01b3ae8..f8d60cb9bb9a 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -174,6 +174,47 @@ static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
 	return true;
 }
 
+/*
+ * Check whether the passed remaining file @size is large enough to contain a
+ * patch of the indicated @patch_size (and also whether this size does not
+ * exceed the per-family maximum).
+ */
+static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int size)
+{
+	u32 max_size;
+
+#define F1XH_MPB_MAX_SIZE 2048
+#define F14H_MPB_MAX_SIZE 1824
+#define F15H_MPB_MAX_SIZE 4096
+#define F16H_MPB_MAX_SIZE 3458
+#define F17H_MPB_MAX_SIZE 3200
+
+	switch (family) {
+	case 0x14:
+		max_size = F14H_MPB_MAX_SIZE;
+		break;
+	case 0x15:
+		max_size = F15H_MPB_MAX_SIZE;
+		break;
+	case 0x16:
+		max_size = F16H_MPB_MAX_SIZE;
+		break;
+	case 0x17:
+		max_size = F17H_MPB_MAX_SIZE;
+		break;
+	default:
+		max_size = F1XH_MPB_MAX_SIZE;
+		break;
+	}
+
+	if (patch_size > min_t(u32, size, max_size)) {
+		pr_err("patch size mismatch\n");
+		return 0;
+	}
+
+	return patch_size;
+}
+
 /*
  * This scans the ucode blob for the proper container as we can have multiple
  * containers glued together. Returns the equivalence ID from the equivalence
@@ -562,47 +603,6 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig)
 	return 0;
 }
 
-/*
- * Check whether the passed remaining file @size is large enough to contain a
- * patch of the indicated @patch_size (and also whether this size does not
- * exceed the per-family maximum).
- */
-static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int size)
-{
-	u32 max_size;
-
-#define F1XH_MPB_MAX_SIZE 2048
-#define F14H_MPB_MAX_SIZE 1824
-#define F15H_MPB_MAX_SIZE 4096
-#define F16H_MPB_MAX_SIZE 3458
-#define F17H_MPB_MAX_SIZE 3200
-
-	switch (family) {
-	case 0x14:
-		max_size = F14H_MPB_MAX_SIZE;
-		break;
-	case 0x15:
-		max_size = F15H_MPB_MAX_SIZE;
-		break;
-	case 0x16:
-		max_size = F16H_MPB_MAX_SIZE;
-		break;
-	case 0x17:
-		max_size = F17H_MPB_MAX_SIZE;
-		break;
-	default:
-		max_size = F1XH_MPB_MAX_SIZE;
-		break;
-	}
-
-	if (patch_size > min_t(u32, size, max_size)) {
-		pr_err("patch size mismatch\n");
-		return 0;
-	}
-
-	return patch_size;
-}
-
 static enum ucode_state apply_microcode_amd(int cpu)
 {
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Move verify_patch_size() up in the file

[tip-bot for Borislav Petkov]

Commit-ID:  3974b68114fed3735f623931e673538d8ab92d26
Gitweb:     https://git.kernel.org/tip/3974b68114fed3735f623931e673538d8ab92d26
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Thu, 28 Jun 2018 19:44:29 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:45:08 +0100

x86/microcode/AMD: Move verify_patch_size() up in the file

... to enable later improvements.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-4-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 82 ++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 36aeb01b3ae8..f8d60cb9bb9a 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -174,6 +174,47 @@ static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
 	return true;
 }
 
+/*
+ * Check whether the passed remaining file @size is large enough to contain a
+ * patch of the indicated @patch_size (and also whether this size does not
+ * exceed the per-family maximum).
+ */
+static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int size)
+{
+	u32 max_size;
+
+#define F1XH_MPB_MAX_SIZE 2048
+#define F14H_MPB_MAX_SIZE 1824
+#define F15H_MPB_MAX_SIZE 4096
+#define F16H_MPB_MAX_SIZE 3458
+#define F17H_MPB_MAX_SIZE 3200
+
+	switch (family) {
+	case 0x14:
+		max_size = F14H_MPB_MAX_SIZE;
+		break;
+	case 0x15:
+		max_size = F15H_MPB_MAX_SIZE;
+		break;
+	case 0x16:
+		max_size = F16H_MPB_MAX_SIZE;
+		break;
+	case 0x17:
+		max_size = F17H_MPB_MAX_SIZE;
+		break;
+	default:
+		max_size = F1XH_MPB_MAX_SIZE;
+		break;
+	}
+
+	if (patch_size > min_t(u32, size, max_size)) {
+		pr_err("patch size mismatch\n");
+		return 0;
+	}
+
+	return patch_size;
+}
+
 /*
  * This scans the ucode blob for the proper container as we can have multiple
  * containers glued together. Returns the equivalence ID from the equivalence
@@ -562,47 +603,6 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig)
 	return 0;
 }
 
-/*
- * Check whether the passed remaining file @size is large enough to contain a
- * patch of the indicated @patch_size (and also whether this size does not
- * exceed the per-family maximum).
- */
-static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int size)
-{
-	u32 max_size;
-
-#define F1XH_MPB_MAX_SIZE 2048
-#define F14H_MPB_MAX_SIZE 1824
-#define F15H_MPB_MAX_SIZE 4096
-#define F16H_MPB_MAX_SIZE 3458
-#define F17H_MPB_MAX_SIZE 3200
-
-	switch (family) {
-	case 0x14:
-		max_size = F14H_MPB_MAX_SIZE;
-		break;
-	case 0x15:
-		max_size = F15H_MPB_MAX_SIZE;
-		break;
-	case 0x16:
-		max_size = F16H_MPB_MAX_SIZE;
-		break;
-	case 0x17:
-		max_size = F17H_MPB_MAX_SIZE;
-		break;
-	default:
-		max_size = F1XH_MPB_MAX_SIZE;
-		break;
-	}
-
-	if (patch_size > min_t(u32, size, max_size)) {
-		pr_err("patch size mismatch\n");
-		return 0;
-	}
-
-	return patch_size;
-}
-
 static enum ucode_state apply_microcode_amd(int cpu)
 {
 	struct cpuinfo_x86 *c = &cpu_data(cpu);

[PATCH 04/16] x86/microcode/AMD: Clean up per-family patch size checks

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Starting with family 0x15, the patch size verification is not needed
anymore. Thus get rid of the need to update this checking function with
each new family.

Keep the check for older families.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/kernel/cpu/microcode/amd.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index f8d60cb9bb9a..bf682deb84e8 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -183,27 +183,22 @@ static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int si
 {
 	u32 max_size;
 
+	if (family >= 0x15)
+		return min_t(u32, patch_size, size);
+
 #define F1XH_MPB_MAX_SIZE 2048
 #define F14H_MPB_MAX_SIZE 1824
-#define F15H_MPB_MAX_SIZE 4096
-#define F16H_MPB_MAX_SIZE 3458
-#define F17H_MPB_MAX_SIZE 3200
 
 	switch (family) {
+	case 0x10 ... 0x12:
+		max_size = F1XH_MPB_MAX_SIZE;
+		break;
 	case 0x14:
 		max_size = F14H_MPB_MAX_SIZE;
 		break;
-	case 0x15:
-		max_size = F15H_MPB_MAX_SIZE;
-		break;
-	case 0x16:
-		max_size = F16H_MPB_MAX_SIZE;
-		break;
-	case 0x17:
-		max_size = F17H_MPB_MAX_SIZE;
-		break;
 	default:
-		max_size = F1XH_MPB_MAX_SIZE;
+		WARN(1, "%s: WTF family: 0x%x\n", __func__, family);
+		return 0;
 		break;
 	}
 
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Clean up per-family patch size checks

[tip-bot for Borislav Petkov]

Commit-ID:  cfffbfeb424bc274aba3b5ed4afd5891662481a8
Gitweb:     https://git.kernel.org/tip/cfffbfeb424bc274aba3b5ed4afd5891662481a8
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Sat, 28 Jul 2018 19:04:02 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:51:00 +0100

x86/microcode/AMD: Clean up per-family patch size checks

Starting with family 0x15, the patch size verification is not needed
anymore. Thus get rid of the need to update this checking function with
each new family.

Keep the check for older families.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-5-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index f8d60cb9bb9a..bf682deb84e8 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -183,27 +183,22 @@ static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int si
 {
 	u32 max_size;
 
+	if (family >= 0x15)
+		return min_t(u32, patch_size, size);
+
 #define F1XH_MPB_MAX_SIZE 2048
 #define F14H_MPB_MAX_SIZE 1824
-#define F15H_MPB_MAX_SIZE 4096
-#define F16H_MPB_MAX_SIZE 3458
-#define F17H_MPB_MAX_SIZE 3200
 
 	switch (family) {
+	case 0x10 ... 0x12:
+		max_size = F1XH_MPB_MAX_SIZE;
+		break;
 	case 0x14:
 		max_size = F14H_MPB_MAX_SIZE;
 		break;
-	case 0x15:
-		max_size = F15H_MPB_MAX_SIZE;
-		break;
-	case 0x16:
-		max_size = F16H_MPB_MAX_SIZE;
-		break;
-	case 0x17:
-		max_size = F17H_MPB_MAX_SIZE;
-		break;
 	default:
-		max_size = F1XH_MPB_MAX_SIZE;
+		WARN(1, "%s: WTF family: 0x%x\n", __func__, family);
+		return 0;
 		break;
 	}
 

[PATCH 05/16] x86/microcode/AMD: Cleanup verify_patch_size() more

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Rename the variable which contains the patch size read out from the
section header to sh_psize for better differentiation of all the "sizes"
in that function.

Also, improve the comment above it.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index bf682deb84e8..a94a15aacfe7 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -175,16 +175,18 @@ static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
 }
 
 /*
- * Check whether the passed remaining file @size is large enough to contain a
- * patch of the indicated @patch_size (and also whether this size does not
- * exceed the per-family maximum).
+ * Check whether the passed remaining file @buf_size is large enough to contain
+ * a patch of the indicated @sh_psize (and also whether this size does not
+ * exceed the per-family maximum). @sh_psize is the size read from the section
+ * header.
  */
-static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int size)
+static unsigned int
+verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 {
 	u32 max_size;
 
 	if (family >= 0x15)
-		return min_t(u32, patch_size, size);
+		return min_t(u32, sh_psize, buf_size);
 
 #define F1XH_MPB_MAX_SIZE 2048
 #define F14H_MPB_MAX_SIZE 1824
@@ -202,12 +204,12 @@ static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int si
 		break;
 	}
 
-	if (patch_size > min_t(u32, size, max_size)) {
+	if (sh_psize > min_t(u32, buf_size, max_size)) {
 		pr_err("patch size mismatch\n");
 		return 0;
 	}
 
-	return patch_size;
+	return sh_psize;
 }
 
 /*
@@ -693,14 +695,14 @@ static void cleanup(void)
  */
 static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 {
+	unsigned int sh_psize, crnt_size, ret;
 	struct microcode_header_amd *mc_hdr;
 	struct ucode_patch *patch;
-	unsigned int patch_size, crnt_size, ret;
 	u32 proc_fam;
 	u16 proc_id;
 
-	patch_size  = *(u32 *)(fw + 4);
-	crnt_size   = patch_size + SECTION_HDR_SIZE;
+	sh_psize    = *(u32 *)(fw + 4);
+	crnt_size   = sh_psize + SECTION_HDR_SIZE;
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
@@ -726,7 +728,7 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	 * but is present in the leftover file length so we need to subtract
 	 * it before passing this value to the function below.
 	 */
-	ret = verify_patch_size(family, patch_size, leftover - SECTION_HDR_SIZE);
+	ret = verify_patch_size(family, sh_psize, leftover - SECTION_HDR_SIZE);
 	if (!ret) {
 		pr_err("Patch-ID 0x%08x: size mismatch.\n", mc_hdr->patch_id);
 		return crnt_size;
@@ -738,7 +740,7 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 		return -EINVAL;
 	}
 
-	patch->data = kmemdup(fw + SECTION_HDR_SIZE, patch_size, GFP_KERNEL);
+	patch->data = kmemdup(fw + SECTION_HDR_SIZE, sh_psize, GFP_KERNEL);
 	if (!patch->data) {
 		pr_err("Patch data allocation failure.\n");
 		kfree(patch);
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Cleanup verify_patch_size() more

[tip-bot for Borislav Petkov]

Commit-ID:  70887cb23eda6bedac73eef227b55e3eb74e0cb7
Gitweb:     https://git.kernel.org/tip/70887cb23eda6bedac73eef227b55e3eb74e0cb7
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Thu, 18 Oct 2018 17:55:45 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:51:05 +0100

x86/microcode/AMD: Cleanup verify_patch_size() more

Rename the variable which contains the patch size read out from the
section header to sh_psize for better differentiation of all the "sizes"
in that function.

Also, improve the comment above it.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-6-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index bf682deb84e8..a94a15aacfe7 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -175,16 +175,18 @@ static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
 }
 
 /*
- * Check whether the passed remaining file @size is large enough to contain a
- * patch of the indicated @patch_size (and also whether this size does not
- * exceed the per-family maximum).
+ * Check whether the passed remaining file @buf_size is large enough to contain
+ * a patch of the indicated @sh_psize (and also whether this size does not
+ * exceed the per-family maximum). @sh_psize is the size read from the section
+ * header.
  */
-static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int size)
+static unsigned int
+verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 {
 	u32 max_size;
 
 	if (family >= 0x15)
-		return min_t(u32, patch_size, size);
+		return min_t(u32, sh_psize, buf_size);
 
 #define F1XH_MPB_MAX_SIZE 2048
 #define F14H_MPB_MAX_SIZE 1824
@@ -202,12 +204,12 @@ static unsigned int verify_patch_size(u8 family, u32 patch_size, unsigned int si
 		break;
 	}
 
-	if (patch_size > min_t(u32, size, max_size)) {
+	if (sh_psize > min_t(u32, buf_size, max_size)) {
 		pr_err("patch size mismatch\n");
 		return 0;
 	}
 
-	return patch_size;
+	return sh_psize;
 }
 
 /*
@@ -693,14 +695,14 @@ static void cleanup(void)
  */
 static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 {
+	unsigned int sh_psize, crnt_size, ret;
 	struct microcode_header_amd *mc_hdr;
 	struct ucode_patch *patch;
-	unsigned int patch_size, crnt_size, ret;
 	u32 proc_fam;
 	u16 proc_id;
 
-	patch_size  = *(u32 *)(fw + 4);
-	crnt_size   = patch_size + SECTION_HDR_SIZE;
+	sh_psize    = *(u32 *)(fw + 4);
+	crnt_size   = sh_psize + SECTION_HDR_SIZE;
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
@@ -726,7 +728,7 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	 * but is present in the leftover file length so we need to subtract
 	 * it before passing this value to the function below.
 	 */
-	ret = verify_patch_size(family, patch_size, leftover - SECTION_HDR_SIZE);
+	ret = verify_patch_size(family, sh_psize, leftover - SECTION_HDR_SIZE);
 	if (!ret) {
 		pr_err("Patch-ID 0x%08x: size mismatch.\n", mc_hdr->patch_id);
 		return crnt_size;
@@ -738,7 +740,7 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 		return -EINVAL;
 	}
 
-	patch->data = kmemdup(fw + SECTION_HDR_SIZE, patch_size, GFP_KERNEL);
+	patch->data = kmemdup(fw + SECTION_HDR_SIZE, sh_psize, GFP_KERNEL);
 	if (!patch->data) {
 		pr_err("Patch data allocation failure.\n");
 		kfree(patch);

[PATCH 06/16] x86/microcode/AMD: Concentrate patch verification

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Add a verify_patch() function which tries to sanity-check many aspects
of a microcode patch supplied by an outside container before attempting
a load.

Prepend all sub-functions' names which verify an aspect of a microcode
patch with "__".

Call it in verify_and_add_patch() *before* looking at the microcode
header.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 79 ++++++++++++++++++++---------
 1 file changed, 54 insertions(+), 25 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index a94a15aacfe7..8f012a7f88c4 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -139,11 +139,15 @@ static bool verify_equivalence_table(const u8 *buf, size_t buf_size, bool early)
  * Check whether there is a valid, non-truncated microcode patch section at the
  * beginning of @buf of size @buf_size. Set @early to use this function in the
  * early path.
+ *
+ * On success, @sh_psize returns the patch size according to the section header,
+ * to the caller.
  */
-static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
+static bool
+__verify_patch_section(const u8 *buf, size_t buf_size, u32 *sh_psize, bool early)
 {
+	u32 p_type, p_size;
 	const u32 *hdr;
-	u32 patch_type, patch_size;
 
 	if (buf_size < SECTION_HDR_SIZE) {
 		if (!early)
@@ -153,24 +157,26 @@ static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
 	}
 
 	hdr = (const u32 *)buf;
-	patch_type = hdr[0];
-	patch_size = hdr[1];
+	p_type = hdr[0];
+	p_size = hdr[1];
 
-	if (patch_type != UCODE_UCODE_TYPE) {
+	if (p_type != UCODE_UCODE_TYPE) {
 		if (!early)
 			pr_debug("Invalid type field (0x%x) in container file section header.\n",
-				patch_type);
+				p_type);
 
 		return false;
 	}
 
-	if (patch_size < sizeof(struct microcode_header_amd)) {
+	if (p_size < sizeof(struct microcode_header_amd)) {
 		if (!early)
-			pr_debug("Patch of size %u too short.\n", patch_size);
+			pr_debug("Patch of size %u too short.\n", p_size);
 
 		return false;
 	}
 
+	*sh_psize = p_size;
+
 	return true;
 }
 
@@ -181,7 +187,7 @@ static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
  * header.
  */
 static unsigned int
-verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
+__verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 {
 	u32 max_size;
 
@@ -212,6 +218,34 @@ verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 	return sh_psize;
 }
 
+static unsigned int
+verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
+{
+	u32 sh_psize;
+
+	if (!__verify_patch_section(buf, buf_size, &sh_psize, early))
+		return 0;
+	/*
+	 * The section header length is not included in this indicated size
+	 * but is present in the leftover file length so we need to subtract
+	 * it before passing this value to the function below.
+	 */
+	buf_size -= SECTION_HDR_SIZE;
+
+	/*
+	 * Check if the remaining buffer is big enough to contain a patch of
+	 * size sh_psize, as the section claims.
+	 */
+	if (buf_size < sh_psize) {
+		if (!early)
+			pr_debug("Patch of size %u truncated.\n", sh_psize);
+
+		return 0;
+	}
+
+	return __verify_patch_size(family, sh_psize, buf_size);
+}
+
 /*
  * This scans the ucode blob for the proper container as we can have multiple
  * containers glued together. Returns the equivalence ID from the equivalence
@@ -687,7 +721,7 @@ static void cleanup(void)
 }
 
 /*
- * We return the current size even if some of the checks failed so that
+ * Return a non-negative value even if some of the checks failed so that
  * we can skip over the next patch. If we return a negative value, we
  * signal a grave error like a memory allocation has failed and the
  * driver cannot continue functioning normally. In such cases, we tear
@@ -695,14 +729,20 @@ static void cleanup(void)
  */
 static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 {
-	unsigned int sh_psize, crnt_size, ret;
 	struct microcode_header_amd *mc_hdr;
+	unsigned int patch_size, crnt_size;
 	struct ucode_patch *patch;
 	u32 proc_fam;
 	u16 proc_id;
 
-	sh_psize    = *(u32 *)(fw + 4);
-	crnt_size   = sh_psize + SECTION_HDR_SIZE;
+	patch_size = verify_patch(family, fw, leftover, false);
+	if (!patch_size) {
+		pr_debug("Patch size mismatch.\n");
+		return 1;
+	}
+
+	/* If initial rough pokes pass, we can start looking at the header. */
+	crnt_size   = patch_size + SECTION_HDR_SIZE;
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
@@ -723,24 +763,13 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 		return crnt_size;
 	}
 
-	/*
-	 * The section header length is not included in this indicated size
-	 * but is present in the leftover file length so we need to subtract
-	 * it before passing this value to the function below.
-	 */
-	ret = verify_patch_size(family, sh_psize, leftover - SECTION_HDR_SIZE);
-	if (!ret) {
-		pr_err("Patch-ID 0x%08x: size mismatch.\n", mc_hdr->patch_id);
-		return crnt_size;
-	}
-
 	patch = kzalloc(sizeof(*patch), GFP_KERNEL);
 	if (!patch) {
 		pr_err("Patch allocation failure.\n");
 		return -EINVAL;
 	}
 
-	patch->data = kmemdup(fw + SECTION_HDR_SIZE, sh_psize, GFP_KERNEL);
+	patch->data = kmemdup(fw + SECTION_HDR_SIZE, patch_size, GFP_KERNEL);
 	if (!patch->data) {
 		pr_err("Patch data allocation failure.\n");
 		kfree(patch);
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Concentrate patch verification

[tip-bot for Borislav Petkov]

Commit-ID:  2b8d34b1ece506a9bbd47b56d266ee020a0c65ac
Gitweb:     https://git.kernel.org/tip/2b8d34b1ece506a9bbd47b56d266ee020a0c65ac
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Thu, 19 Jul 2018 15:32:42 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:51:05 +0100

x86/microcode/AMD: Concentrate patch verification

Add a verify_patch() function which tries to sanity-check many aspects
of a microcode patch supplied by an outside container before attempting
a load.

Prepend all sub-functions' names which verify an aspect of a microcode
patch with "__".

Call it in verify_and_add_patch() *before* looking at the microcode
header.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-7-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 79 +++++++++++++++++++++++++------------
 1 file changed, 54 insertions(+), 25 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index a94a15aacfe7..8f012a7f88c4 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -139,11 +139,15 @@ static bool verify_equivalence_table(const u8 *buf, size_t buf_size, bool early)
  * Check whether there is a valid, non-truncated microcode patch section at the
  * beginning of @buf of size @buf_size. Set @early to use this function in the
  * early path.
+ *
+ * On success, @sh_psize returns the patch size according to the section header,
+ * to the caller.
  */
-static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
+static bool
+__verify_patch_section(const u8 *buf, size_t buf_size, u32 *sh_psize, bool early)
 {
+	u32 p_type, p_size;
 	const u32 *hdr;
-	u32 patch_type, patch_size;
 
 	if (buf_size < SECTION_HDR_SIZE) {
 		if (!early)
@@ -153,24 +157,26 @@ static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
 	}
 
 	hdr = (const u32 *)buf;
-	patch_type = hdr[0];
-	patch_size = hdr[1];
+	p_type = hdr[0];
+	p_size = hdr[1];
 
-	if (patch_type != UCODE_UCODE_TYPE) {
+	if (p_type != UCODE_UCODE_TYPE) {
 		if (!early)
 			pr_debug("Invalid type field (0x%x) in container file section header.\n",
-				patch_type);
+				p_type);
 
 		return false;
 	}
 
-	if (patch_size < sizeof(struct microcode_header_amd)) {
+	if (p_size < sizeof(struct microcode_header_amd)) {
 		if (!early)
-			pr_debug("Patch of size %u too short.\n", patch_size);
+			pr_debug("Patch of size %u too short.\n", p_size);
 
 		return false;
 	}
 
+	*sh_psize = p_size;
+
 	return true;
 }
 
@@ -181,7 +187,7 @@ static bool verify_patch_section(const u8 *buf, size_t buf_size, bool early)
  * header.
  */
 static unsigned int
-verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
+__verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 {
 	u32 max_size;
 
@@ -212,6 +218,34 @@ verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 	return sh_psize;
 }
 
+static unsigned int
+verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
+{
+	u32 sh_psize;
+
+	if (!__verify_patch_section(buf, buf_size, &sh_psize, early))
+		return 0;
+	/*
+	 * The section header length is not included in this indicated size
+	 * but is present in the leftover file length so we need to subtract
+	 * it before passing this value to the function below.
+	 */
+	buf_size -= SECTION_HDR_SIZE;
+
+	/*
+	 * Check if the remaining buffer is big enough to contain a patch of
+	 * size sh_psize, as the section claims.
+	 */
+	if (buf_size < sh_psize) {
+		if (!early)
+			pr_debug("Patch of size %u truncated.\n", sh_psize);
+
+		return 0;
+	}
+
+	return __verify_patch_size(family, sh_psize, buf_size);
+}
+
 /*
  * This scans the ucode blob for the proper container as we can have multiple
  * containers glued together. Returns the equivalence ID from the equivalence
@@ -687,7 +721,7 @@ static void cleanup(void)
 }
 
 /*
- * We return the current size even if some of the checks failed so that
+ * Return a non-negative value even if some of the checks failed so that
  * we can skip over the next patch. If we return a negative value, we
  * signal a grave error like a memory allocation has failed and the
  * driver cannot continue functioning normally. In such cases, we tear
@@ -695,14 +729,20 @@ static void cleanup(void)
  */
 static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 {
-	unsigned int sh_psize, crnt_size, ret;
 	struct microcode_header_amd *mc_hdr;
+	unsigned int patch_size, crnt_size;
 	struct ucode_patch *patch;
 	u32 proc_fam;
 	u16 proc_id;
 
-	sh_psize    = *(u32 *)(fw + 4);
-	crnt_size   = sh_psize + SECTION_HDR_SIZE;
+	patch_size = verify_patch(family, fw, leftover, false);
+	if (!patch_size) {
+		pr_debug("Patch size mismatch.\n");
+		return 1;
+	}
+
+	/* If initial rough pokes pass, we can start looking at the header. */
+	crnt_size   = patch_size + SECTION_HDR_SIZE;
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
@@ -723,24 +763,13 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 		return crnt_size;
 	}
 
-	/*
-	 * The section header length is not included in this indicated size
-	 * but is present in the leftover file length so we need to subtract
-	 * it before passing this value to the function below.
-	 */
-	ret = verify_patch_size(family, sh_psize, leftover - SECTION_HDR_SIZE);
-	if (!ret) {
-		pr_err("Patch-ID 0x%08x: size mismatch.\n", mc_hdr->patch_id);
-		return crnt_size;
-	}
-
 	patch = kzalloc(sizeof(*patch), GFP_KERNEL);
 	if (!patch) {
 		pr_err("Patch allocation failure.\n");
 		return -EINVAL;
 	}
 
-	patch->data = kmemdup(fw + SECTION_HDR_SIZE, sh_psize, GFP_KERNEL);
+	patch->data = kmemdup(fw + SECTION_HDR_SIZE, patch_size, GFP_KERNEL);
 	if (!patch->data) {
 		pr_err("Patch data allocation failure.\n");
 		kfree(patch);

[PATCH 07/16] x86/microcode/AMD: Simplify patch family detection

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Instead of traversing the equivalence table, compute the family a patch
is for, from the processor revision ID in the microcode header.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 8f012a7f88c4..b4450374f4b1 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -540,20 +540,6 @@ static u16 __find_equiv_id(unsigned int cpu)
 	return find_equiv_id(equiv_cpu_table, uci->cpu_sig.sig);
 }
 
-static u32 find_cpu_family_by_equiv_cpu(u16 equiv_cpu)
-{
-	int i = 0;
-
-	BUG_ON(!equiv_cpu_table);
-
-	while (equiv_cpu_table[i].equiv_cpu != 0) {
-		if (equiv_cpu == equiv_cpu_table[i].equiv_cpu)
-			return equiv_cpu_table[i].installed_cpu;
-		i++;
-	}
-	return 0;
-}
-
 /*
  * a small, trivial cache of per-family ucode patches
  */
@@ -732,7 +718,7 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	struct microcode_header_amd *mc_hdr;
 	unsigned int patch_size, crnt_size;
 	struct ucode_patch *patch;
-	u32 proc_fam;
+	u8 patch_fam;
 	u16 proc_id;
 
 	patch_size = verify_patch(family, fw, leftover, false);
@@ -746,15 +732,8 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
-	proc_fam = find_cpu_family_by_equiv_cpu(proc_id);
-	if (!proc_fam) {
-		pr_err("No patch family for equiv ID: 0x%04x\n", proc_id);
-		return crnt_size;
-	}
-
-	/* check if patch is for the current family */
-	proc_fam = ((proc_fam >> 8) & 0xf) + ((proc_fam >> 20) & 0xff);
-	if (proc_fam != family)
+	patch_fam = 0xf + (proc_id >> 12);
+	if (patch_fam != family)
 		return crnt_size;
 
 	if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) {
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Simplify patch family detection

[tip-bot for Borislav Petkov]

Commit-ID:  6cdce951f7a1d8161910d5cd9bd9e6197bf5cc97
Gitweb:     https://git.kernel.org/tip/6cdce951f7a1d8161910d5cd9bd9e6197bf5cc97
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Thu, 19 Jul 2018 15:47:07 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:51:05 +0100

x86/microcode/AMD: Simplify patch family detection

Instead of traversing the equivalence table, compute the family a patch
is for, from the processor revision ID in the microcode header.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-8-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 8f012a7f88c4..b4450374f4b1 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -540,20 +540,6 @@ static u16 __find_equiv_id(unsigned int cpu)
 	return find_equiv_id(equiv_cpu_table, uci->cpu_sig.sig);
 }
 
-static u32 find_cpu_family_by_equiv_cpu(u16 equiv_cpu)
-{
-	int i = 0;
-
-	BUG_ON(!equiv_cpu_table);
-
-	while (equiv_cpu_table[i].equiv_cpu != 0) {
-		if (equiv_cpu == equiv_cpu_table[i].equiv_cpu)
-			return equiv_cpu_table[i].installed_cpu;
-		i++;
-	}
-	return 0;
-}
-
 /*
  * a small, trivial cache of per-family ucode patches
  */
@@ -732,7 +718,7 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	struct microcode_header_amd *mc_hdr;
 	unsigned int patch_size, crnt_size;
 	struct ucode_patch *patch;
-	u32 proc_fam;
+	u8 patch_fam;
 	u16 proc_id;
 
 	patch_size = verify_patch(family, fw, leftover, false);
@@ -746,15 +732,8 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
-	proc_fam = find_cpu_family_by_equiv_cpu(proc_id);
-	if (!proc_fam) {
-		pr_err("No patch family for equiv ID: 0x%04x\n", proc_id);
-		return crnt_size;
-	}
-
-	/* check if patch is for the current family */
-	proc_fam = ((proc_fam >> 8) & 0xf) + ((proc_fam >> 20) & 0xff);
-	if (proc_fam != family)
+	patch_fam = 0xf + (proc_id >> 12);
+	if (patch_fam != family)
 		return crnt_size;
 
 	if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) {

[PATCH 08/16] x86/microcode/AMD: Move patch family check to verify_patch()

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

... where all the microcode patch verification is being concentrated.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index b4450374f4b1..be46e1fda77f 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -221,7 +221,10 @@ __verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 static unsigned int
 verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
 {
+	struct microcode_header_amd *mc_hdr;
 	u32 sh_psize;
+	u16 proc_id;
+	u8 patch_fam;
 
 	if (!__verify_patch_section(buf, buf_size, &sh_psize, early))
 		return 0;
@@ -243,6 +246,13 @@ verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
 		return 0;
 	}
 
+	mc_hdr	= (struct microcode_header_amd *)(buf + SECTION_HDR_SIZE);
+	proc_id	= mc_hdr->processor_rev_id;
+
+	patch_fam = 0xf + (proc_id >> 12);
+	if (patch_fam != family)
+		return 0;
+
 	return __verify_patch_size(family, sh_psize, buf_size);
 }
 
@@ -718,7 +728,6 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	struct microcode_header_amd *mc_hdr;
 	unsigned int patch_size, crnt_size;
 	struct ucode_patch *patch;
-	u8 patch_fam;
 	u16 proc_id;
 
 	patch_size = verify_patch(family, fw, leftover, false);
@@ -732,10 +741,6 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
-	patch_fam = 0xf + (proc_id >> 12);
-	if (patch_fam != family)
-		return crnt_size;
-
 	if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) {
 		pr_err("Patch-ID 0x%08x: chipset-specific code unsupported.\n",
 			mc_hdr->patch_id);
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Move patch family check to verify_patch()

[tip-bot for Borislav Petkov]

Commit-ID:  51776fb805fe4f248f264c1783c286ae60552e9a
Gitweb:     https://git.kernel.org/tip/51776fb805fe4f248f264c1783c286ae60552e9a
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Thu, 26 Jul 2018 06:50:44 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:51:05 +0100

x86/microcode/AMD: Move patch family check to verify_patch()

... where all the microcode patch verification is being concentrated.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-9-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index b4450374f4b1..be46e1fda77f 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -221,7 +221,10 @@ __verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 static unsigned int
 verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
 {
+	struct microcode_header_amd *mc_hdr;
 	u32 sh_psize;
+	u16 proc_id;
+	u8 patch_fam;
 
 	if (!__verify_patch_section(buf, buf_size, &sh_psize, early))
 		return 0;
@@ -243,6 +246,13 @@ verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
 		return 0;
 	}
 
+	mc_hdr	= (struct microcode_header_amd *)(buf + SECTION_HDR_SIZE);
+	proc_id	= mc_hdr->processor_rev_id;
+
+	patch_fam = 0xf + (proc_id >> 12);
+	if (patch_fam != family)
+		return 0;
+
 	return __verify_patch_size(family, sh_psize, buf_size);
 }
 
@@ -718,7 +728,6 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	struct microcode_header_amd *mc_hdr;
 	unsigned int patch_size, crnt_size;
 	struct ucode_patch *patch;
-	u8 patch_fam;
 	u16 proc_id;
 
 	patch_size = verify_patch(family, fw, leftover, false);
@@ -732,10 +741,6 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
-	patch_fam = 0xf + (proc_id >> 12);
-	if (patch_fam != family)
-		return crnt_size;
-
 	if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) {
 		pr_err("Patch-ID 0x%08x: chipset-specific code unsupported.\n",
 			mc_hdr->patch_id);

[PATCH 09/16] x86/microcode/AMD: Move chipset-specific check into verify_patch()

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

... where it belongs.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index be46e1fda77f..ebac82357639 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -249,6 +249,12 @@ verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
 	mc_hdr	= (struct microcode_header_amd *)(buf + SECTION_HDR_SIZE);
 	proc_id	= mc_hdr->processor_rev_id;
 
+	if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) {
+		if (!early)
+			pr_err("Patch-ID 0x%08x: chipset-specific code unsupported.\n", mc_hdr->patch_id);
+		return 0;
+	}
+
 	patch_fam = 0xf + (proc_id >> 12);
 	if (patch_fam != family)
 		return 0;
@@ -741,12 +747,6 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
-	if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) {
-		pr_err("Patch-ID 0x%08x: chipset-specific code unsupported.\n",
-			mc_hdr->patch_id);
-		return crnt_size;
-	}
-
 	patch = kzalloc(sizeof(*patch), GFP_KERNEL);
 	if (!patch) {
 		pr_err("Patch allocation failure.\n");
-- 
2.19.1

[PATCH 10/16] x86/microcode/AMD: Change verify_patch()'s return value

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Have it return 0 on success, positive value when the current patch
should be skipped and negative on error.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 91 ++++++++++++++++-------------
 1 file changed, 52 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index ebac82357639..79216cfb9f72 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -210,24 +210,32 @@ __verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 		break;
 	}
 
-	if (sh_psize > min_t(u32, buf_size, max_size)) {
-		pr_err("patch size mismatch\n");
+	if (sh_psize > min_t(u32, buf_size, max_size))
 		return 0;
-	}
 
 	return sh_psize;
 }
 
-static unsigned int
-verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
+/*
+ * Verify the patch in @buf.
+ *
+ * Returns:
+ * negative: on error
+ * positive: patch is not for this family, skip it
+ * 0: success
+ */
+static int
+verify_patch(u8 family, const u8 *buf, size_t buf_size, u32 *patch_size, bool early)
 {
 	struct microcode_header_amd *mc_hdr;
+	unsigned int ret;
 	u32 sh_psize;
 	u16 proc_id;
 	u8 patch_fam;
 
 	if (!__verify_patch_section(buf, buf_size, &sh_psize, early))
-		return 0;
+		return -1;
+
 	/*
 	 * The section header length is not included in this indicated size
 	 * but is present in the leftover file length so we need to subtract
@@ -243,23 +251,31 @@ verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
 		if (!early)
 			pr_debug("Patch of size %u truncated.\n", sh_psize);
 
-		return 0;
+		return -1;
 	}
 
-	mc_hdr	= (struct microcode_header_amd *)(buf + SECTION_HDR_SIZE);
-	proc_id	= mc_hdr->processor_rev_id;
+	ret = __verify_patch_size(family, sh_psize, buf_size);
+	if (!ret) {
+		if (!early)
+			pr_debug("Per-family patch size mismatch.\n");
+		return -1;
+	}
+
+	*patch_size = sh_psize;
 
+	mc_hdr	= (struct microcode_header_amd *)(buf + SECTION_HDR_SIZE);
 	if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) {
 		if (!early)
 			pr_err("Patch-ID 0x%08x: chipset-specific code unsupported.\n", mc_hdr->patch_id);
-		return 0;
+		return -1;
 	}
 
+	proc_id	= mc_hdr->processor_rev_id;
 	patch_fam = 0xf + (proc_id >> 12);
 	if (patch_fam != family)
-		return 0;
+		return 1;
 
-	return __verify_patch_size(family, sh_psize, buf_size);
+	return 0;
 }
 
 /*
@@ -729,23 +745,17 @@ static void cleanup(void)
  * driver cannot continue functioning normally. In such cases, we tear
  * down everything we've used up so far and exit.
  */
-static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
+static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover,
+				unsigned int *patch_size)
 {
 	struct microcode_header_amd *mc_hdr;
-	unsigned int patch_size, crnt_size;
 	struct ucode_patch *patch;
 	u16 proc_id;
+	int ret;
 
-	patch_size = verify_patch(family, fw, leftover, false);
-	if (!patch_size) {
-		pr_debug("Patch size mismatch.\n");
-		return 1;
-	}
-
-	/* If initial rough pokes pass, we can start looking at the header. */
-	crnt_size   = patch_size + SECTION_HDR_SIZE;
-	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
-	proc_id	    = mc_hdr->processor_rev_id;
+	ret = verify_patch(family, fw, leftover, patch_size, false);
+	if (ret)
+		return ret;
 
 	patch = kzalloc(sizeof(*patch), GFP_KERNEL);
 	if (!patch) {
@@ -753,13 +763,16 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 		return -EINVAL;
 	}
 
-	patch->data = kmemdup(fw + SECTION_HDR_SIZE, patch_size, GFP_KERNEL);
+	patch->data = kmemdup(fw + SECTION_HDR_SIZE, *patch_size, GFP_KERNEL);
 	if (!patch->data) {
 		pr_err("Patch data allocation failure.\n");
 		kfree(patch);
 		return -EINVAL;
 	}
 
+	mc_hdr      = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
+	proc_id     = mc_hdr->processor_rev_id;
+
 	INIT_LIST_HEAD(&patch->plist);
 	patch->patch_id  = mc_hdr->patch_id;
 	patch->equiv_cpu = proc_id;
@@ -770,39 +783,39 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	/* ... and add to cache. */
 	update_cache(patch);
 
-	return crnt_size;
+	return 0;
 }
 
 static enum ucode_state __load_microcode_amd(u8 family, const u8 *data,
 					     size_t size)
 {
-	enum ucode_state ret = UCODE_ERROR;
-	unsigned int leftover;
 	u8 *fw = (u8 *)data;
-	int crnt_size = 0;
 	int offset;
 
 	offset = install_equiv_cpu_table(data);
 	if (offset < 0) {
 		pr_err("failed to create equivalent cpu table\n");
-		return ret;
+		return UCODE_ERROR;
 	}
-	fw += offset;
-	leftover = size - offset;
+	fw   += offset;
+	size -= offset;
 
 	if (*(u32 *)fw != UCODE_UCODE_TYPE) {
 		pr_err("invalid type field in container file section header\n");
 		free_equiv_cpu_table();
-		return ret;
+		return UCODE_ERROR;
 	}
 
-	while (leftover) {
-		crnt_size = verify_and_add_patch(family, fw, leftover);
-		if (crnt_size < 0)
-			return ret;
+	while (size > 0) {
+		unsigned int crnt_size = 0;
+		int ret;
+
+		ret = verify_and_add_patch(family, fw, size, &crnt_size);
+		if (ret < 0)
+			return UCODE_ERROR;
 
-		fw	 += crnt_size;
-		leftover -= crnt_size;
+		fw   +=  crnt_size + SECTION_HDR_SIZE;
+		size -= (crnt_size + SECTION_HDR_SIZE);
 	}
 
 	return UCODE_OK;
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Change verify_patch()'s return value

[tip-bot for Borislav Petkov]

Commit-ID:  d430a305b7f8127d992e79e90b6fac00f99d23b3
Gitweb:     https://git.kernel.org/tip/d430a305b7f8127d992e79e90b6fac00f99d23b3
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Fri, 19 Oct 2018 14:27:24 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:51:06 +0100

x86/microcode/AMD: Change verify_patch()'s return value

Have it return 0 on success, positive value when the current patch
should be skipped and negative on error.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-11-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 91 +++++++++++++++++++++----------------
 1 file changed, 52 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index ebac82357639..79216cfb9f72 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -210,24 +210,32 @@ __verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
 		break;
 	}
 
-	if (sh_psize > min_t(u32, buf_size, max_size)) {
-		pr_err("patch size mismatch\n");
+	if (sh_psize > min_t(u32, buf_size, max_size))
 		return 0;
-	}
 
 	return sh_psize;
 }
 
-static unsigned int
-verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
+/*
+ * Verify the patch in @buf.
+ *
+ * Returns:
+ * negative: on error
+ * positive: patch is not for this family, skip it
+ * 0: success
+ */
+static int
+verify_patch(u8 family, const u8 *buf, size_t buf_size, u32 *patch_size, bool early)
 {
 	struct microcode_header_amd *mc_hdr;
+	unsigned int ret;
 	u32 sh_psize;
 	u16 proc_id;
 	u8 patch_fam;
 
 	if (!__verify_patch_section(buf, buf_size, &sh_psize, early))
-		return 0;
+		return -1;
+
 	/*
 	 * The section header length is not included in this indicated size
 	 * but is present in the leftover file length so we need to subtract
@@ -243,23 +251,31 @@ verify_patch(u8 family, const u8 *buf, unsigned int buf_size, bool early)
 		if (!early)
 			pr_debug("Patch of size %u truncated.\n", sh_psize);
 
-		return 0;
+		return -1;
 	}
 
-	mc_hdr	= (struct microcode_header_amd *)(buf + SECTION_HDR_SIZE);
-	proc_id	= mc_hdr->processor_rev_id;
+	ret = __verify_patch_size(family, sh_psize, buf_size);
+	if (!ret) {
+		if (!early)
+			pr_debug("Per-family patch size mismatch.\n");
+		return -1;
+	}
+
+	*patch_size = sh_psize;
 
+	mc_hdr	= (struct microcode_header_amd *)(buf + SECTION_HDR_SIZE);
 	if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) {
 		if (!early)
 			pr_err("Patch-ID 0x%08x: chipset-specific code unsupported.\n", mc_hdr->patch_id);
-		return 0;
+		return -1;
 	}
 
+	proc_id	= mc_hdr->processor_rev_id;
 	patch_fam = 0xf + (proc_id >> 12);
 	if (patch_fam != family)
-		return 0;
+		return 1;
 
-	return __verify_patch_size(family, sh_psize, buf_size);
+	return 0;
 }
 
 /*
@@ -729,23 +745,17 @@ static void cleanup(void)
  * driver cannot continue functioning normally. In such cases, we tear
  * down everything we've used up so far and exit.
  */
-static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
+static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover,
+				unsigned int *patch_size)
 {
 	struct microcode_header_amd *mc_hdr;
-	unsigned int patch_size, crnt_size;
 	struct ucode_patch *patch;
 	u16 proc_id;
+	int ret;
 
-	patch_size = verify_patch(family, fw, leftover, false);
-	if (!patch_size) {
-		pr_debug("Patch size mismatch.\n");
-		return 1;
-	}
-
-	/* If initial rough pokes pass, we can start looking at the header. */
-	crnt_size   = patch_size + SECTION_HDR_SIZE;
-	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
-	proc_id	    = mc_hdr->processor_rev_id;
+	ret = verify_patch(family, fw, leftover, patch_size, false);
+	if (ret)
+		return ret;
 
 	patch = kzalloc(sizeof(*patch), GFP_KERNEL);
 	if (!patch) {
@@ -753,13 +763,16 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 		return -EINVAL;
 	}
 
-	patch->data = kmemdup(fw + SECTION_HDR_SIZE, patch_size, GFP_KERNEL);
+	patch->data = kmemdup(fw + SECTION_HDR_SIZE, *patch_size, GFP_KERNEL);
 	if (!patch->data) {
 		pr_err("Patch data allocation failure.\n");
 		kfree(patch);
 		return -EINVAL;
 	}
 
+	mc_hdr      = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
+	proc_id     = mc_hdr->processor_rev_id;
+
 	INIT_LIST_HEAD(&patch->plist);
 	patch->patch_id  = mc_hdr->patch_id;
 	patch->equiv_cpu = proc_id;
@@ -770,39 +783,39 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	/* ... and add to cache. */
 	update_cache(patch);
 
-	return crnt_size;
+	return 0;
 }
 
 static enum ucode_state __load_microcode_amd(u8 family, const u8 *data,
 					     size_t size)
 {
-	enum ucode_state ret = UCODE_ERROR;
-	unsigned int leftover;
 	u8 *fw = (u8 *)data;
-	int crnt_size = 0;
 	int offset;
 
 	offset = install_equiv_cpu_table(data);
 	if (offset < 0) {
 		pr_err("failed to create equivalent cpu table\n");
-		return ret;
+		return UCODE_ERROR;
 	}
-	fw += offset;
-	leftover = size - offset;
+	fw   += offset;
+	size -= offset;
 
 	if (*(u32 *)fw != UCODE_UCODE_TYPE) {
 		pr_err("invalid type field in container file section header\n");
 		free_equiv_cpu_table();
-		return ret;
+		return UCODE_ERROR;
 	}
 
-	while (leftover) {
-		crnt_size = verify_and_add_patch(family, fw, leftover);
-		if (crnt_size < 0)
-			return ret;
+	while (size > 0) {
+		unsigned int crnt_size = 0;
+		int ret;
+
+		ret = verify_and_add_patch(family, fw, size, &crnt_size);
+		if (ret < 0)
+			return UCODE_ERROR;
 
-		fw	 += crnt_size;
-		leftover -= crnt_size;
+		fw   +=  crnt_size + SECTION_HDR_SIZE;
+		size -= (crnt_size + SECTION_HDR_SIZE);
 	}
 
 	return UCODE_OK;

[PATCH 11/16] x86/microcode/AMD: Convert early parser to the new verification routines

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Now that they have the required functionality, use them to verify the
equivalence table and each patch, thus making parse_container() more
readable.

Based on a patch by "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 46 +++++++++++++++--------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 79216cfb9f72..5775dc996df3 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -293,17 +293,18 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	u16 eq_id;
 	u8 *buf;
 
-	/* 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;
+	if (!verify_equivalence_table(ucode, size, true))
+		return 0;
 
 	buf = ucode;
 
 	eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
-	/* Find the equivalence ID of our CPU in this table: */
+	/*
+	 * Find the equivalence ID of our CPU in this table. Even if this table
+	 * doesn't contain a patch for the CPU, scan through the whole container
+	 * so that it can be skipped in case there are other containers appended.
+	 */
 	eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
 
 	buf  += hdr[2] + CONTAINER_HDR_SZ;
@@ -316,29 +317,29 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	while (size > 0) {
 		struct microcode_amd *mc;
 		u32 patch_size;
+		int ret;
 
-		hdr = (u32 *)buf;
-
-		if (hdr[0] != UCODE_UCODE_TYPE)
-			break;
-
-		/* Sanity-check patch size. */
-		patch_size = hdr[1];
-		if (patch_size > PATCH_MAX_SIZE)
-			break;
-
-		/* Skip patch section header: */
-		buf  += SECTION_HDR_SIZE;
-		size -= SECTION_HDR_SIZE;
+		ret = verify_patch(x86_family(desc->cpuid_1_eax), buf, size, &patch_size, true);
+		if (ret < 0) {
+			/*
+			 * Patch verification failed, skip to the next
+			 * container, if there's one:
+			 */
+			goto out;
+		} else if (ret > 0) {
+			goto skip;
+		}
 
-		mc = (struct microcode_amd *)buf;
+		mc = (struct microcode_amd *)(buf + SECTION_HDR_SIZE);
 		if (eq_id == mc->hdr.processor_rev_id) {
 			desc->psize = patch_size;
 			desc->mc = mc;
 		}
 
-		buf  += patch_size;
-		size -= patch_size;
+skip:
+		/* Skip patch section header too: */
+		buf  += patch_size + SECTION_HDR_SIZE;
+		size -= patch_size + SECTION_HDR_SIZE;
 	}
 
 	/*
@@ -355,6 +356,7 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 		return 0;
 	}
 
+out:
 	return orig_size - size;
 }
 
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Convert early parser to the new verification routines

[tip-bot for Borislav Petkov]

Commit-ID:  c45e80358cb3c61eaf6f5d35af86c6a874935bd0
Gitweb:     https://git.kernel.org/tip/c45e80358cb3c61eaf6f5d35af86c6a874935bd0
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Wed, 12 Sep 2018 18:33:34 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:51:06 +0100

x86/microcode/AMD: Convert early parser to the new verification routines

Now that they have the required functionality, use them to verify the
equivalence table and each patch, thus making parse_container() more
readable.

Originally-by: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-12-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 46 +++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 79216cfb9f72..5775dc996df3 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -293,17 +293,18 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	u16 eq_id;
 	u8 *buf;
 
-	/* 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;
+	if (!verify_equivalence_table(ucode, size, true))
+		return 0;
 
 	buf = ucode;
 
 	eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
-	/* Find the equivalence ID of our CPU in this table: */
+	/*
+	 * Find the equivalence ID of our CPU in this table. Even if this table
+	 * doesn't contain a patch for the CPU, scan through the whole container
+	 * so that it can be skipped in case there are other containers appended.
+	 */
 	eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
 
 	buf  += hdr[2] + CONTAINER_HDR_SZ;
@@ -316,29 +317,29 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	while (size > 0) {
 		struct microcode_amd *mc;
 		u32 patch_size;
+		int ret;
 
-		hdr = (u32 *)buf;
-
-		if (hdr[0] != UCODE_UCODE_TYPE)
-			break;
-
-		/* Sanity-check patch size. */
-		patch_size = hdr[1];
-		if (patch_size > PATCH_MAX_SIZE)
-			break;
-
-		/* Skip patch section header: */
-		buf  += SECTION_HDR_SIZE;
-		size -= SECTION_HDR_SIZE;
+		ret = verify_patch(x86_family(desc->cpuid_1_eax), buf, size, &patch_size, true);
+		if (ret < 0) {
+			/*
+			 * Patch verification failed, skip to the next
+			 * container, if there's one:
+			 */
+			goto out;
+		} else if (ret > 0) {
+			goto skip;
+		}
 
-		mc = (struct microcode_amd *)buf;
+		mc = (struct microcode_amd *)(buf + SECTION_HDR_SIZE);
 		if (eq_id == mc->hdr.processor_rev_id) {
 			desc->psize = patch_size;
 			desc->mc = mc;
 		}
 
-		buf  += patch_size;
-		size -= patch_size;
+skip:
+		/* Skip patch section header too: */
+		buf  += patch_size + SECTION_HDR_SIZE;
+		size -= patch_size + SECTION_HDR_SIZE;
 	}
 
 	/*
@@ -355,6 +356,7 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 		return 0;
 	}
 
+out:
 	return orig_size - size;
 }
 

[PATCH 12/16] x86/microcode/AMD: Fix container size's type

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Make it size_t everywhere as this is what we get from cpio.

Based on a patch by Maciej S. Szmigiero <mail@maciej.szmigiero.name>.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 5775dc996df3..1f9a4b41218e 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -186,8 +186,7 @@ __verify_patch_section(const u8 *buf, size_t buf_size, u32 *sh_psize, bool early
  * exceed the per-family maximum). @sh_psize is the size read from the section
  * header.
  */
-static unsigned int
-__verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
+static unsigned int __verify_patch_size(u8 family, u32 sh_psize, size_t buf_size)
 {
 	u32 max_size;
 
@@ -285,10 +284,10 @@ verify_patch(u8 family, const u8 *buf, size_t buf_size, u32 *patch_size, bool ea
  * 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;
 	u16 eq_id;
 	u8 *buf;
@@ -366,15 +365,17 @@ 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;
+		/* catch wraparound */
+		if (size >= s) {
+			ucode += s;
+			size  -= s;
+		} else
+			return;
 	}
 }
 
-- 
2.19.1

Re: [PATCH 12/16] x86/microcode/AMD: Fix container size's type

[kbuild test robot]

Hi Borislav,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tip/x86/core]
[also build test WARNING on v4.20-rc1 next-20181109]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Borislav-Petkov/x86-microcode-AMD-Improve-container-verification/20181109-065233

smatch warnings:
arch/x86/kernel/cpu/microcode/amd.c:368 scan_containers() warn: always true condition '(size >= 0) => (0-u32max >= 0)'
arch/x86/kernel/cpu/microcode/amd.c:368 scan_containers() warn: always true condition '(size >= 0) => (0-u32max >= 0)'

vim +368 arch/x86/kernel/cpu/microcode/amd.c

   361	
   362	/*
   363	 * Scan the ucode blob for the proper container as we can have multiple
   364	 * containers glued together.
   365	 */
   366	static void scan_containers(u8 *ucode, size_t size, struct cont_desc *desc)
   367	{
 > 368		while (size >= 0) {
   369			size_t s = parse_container(ucode, size, desc);
   370			if (!s)
   371				return;
   372	
   373			/* catch wraparound */
   374			if (size >= s) {
   375				ucode += s;
   376				size  -= s;
   377			} else
   378				return;
   379		}
   380	}
   381	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[tip:x86/microcode] x86/microcode/AMD: Fix container size's type

[tip-bot for Borislav Petkov]

Commit-ID:  72dc571a3a77081112944fe0c2dbff614114b04a
Gitweb:     https://git.kernel.org/tip/72dc571a3a77081112944fe0c2dbff614114b04a
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Wed, 12 Sep 2018 18:50:23 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:55:06 +0100

x86/microcode/AMD: Fix container size's type

Make it size_t everywhere as this is what we get from cpio.

 [ bp: Fix a smatch warning. ]

Originally-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/20181107170218.7596-13-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 5775dc996df3..66490ff087d9 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -186,8 +186,7 @@ __verify_patch_section(const u8 *buf, size_t buf_size, u32 *sh_psize, bool early
  * exceed the per-family maximum). @sh_psize is the size read from the section
  * header.
  */
-static unsigned int
-__verify_patch_size(u8 family, u32 sh_psize, unsigned int buf_size)
+static unsigned int __verify_patch_size(u8 family, u32 sh_psize, size_t buf_size)
 {
 	u32 max_size;
 
@@ -285,10 +284,10 @@ verify_patch(u8 family, const u8 *buf, size_t buf_size, u32 *patch_size, bool ea
  * 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;
 	u16 eq_id;
 	u8 *buf;
@@ -366,15 +365,18 @@ out:
  */
 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) {
+		size_t s = parse_container(ucode, size, desc);
 		if (!s)
 			return;
 
-		ucode += s;
-		rem   -= s;
+		/* catch wraparound */
+		if (size >= s) {
+			ucode += s;
+			size  -= s;
+		} else {
+			return;
+		}
 	}
 }
 

[PATCH 13/16] x86/microcode/AMD: Check microcode container data in the late loader

[Borislav Petkov]

From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>

Convert the late loading path to use the newly introduced microcode
container data checking functions as it was previously done for the
early loader.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
[ Keep header length addition in install_equiv_cpu_table() and rediff. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 38 +++++++++++++----------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 1f9a4b41218e..5d822ed295b5 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -705,28 +705,27 @@ static enum ucode_state apply_microcode_amd(int cpu)
 	return ret;
 }
 
-static int install_equiv_cpu_table(const u8 *buf)
+static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 {
-	unsigned int *ibuf = (unsigned int *)buf;
-	unsigned int type = ibuf[1];
-	unsigned int size = ibuf[2];
+	u32 equiv_tbl_len;
+	const u32 *hdr;
 
-	if (type != UCODE_EQUIV_CPU_TABLE_TYPE || !size) {
-		pr_err("empty section/"
-		       "invalid type field in container file section header\n");
-		return -EINVAL;
-	}
+	if (!verify_equivalence_table(buf, buf_size, false))
+		return 0;
 
-	equiv_cpu_table = vmalloc(size);
+	hdr = (const u32 *)buf;
+	equiv_tbl_len = hdr[2];
+
+	equiv_cpu_table = vmalloc(equiv_tbl_len);
 	if (!equiv_cpu_table) {
 		pr_err("failed to allocate equivalent CPU table\n");
-		return -ENOMEM;
+		return 0;
 	}
 
-	memcpy(equiv_cpu_table, buf + CONTAINER_HDR_SZ, size);
+	memcpy(equiv_cpu_table, buf + CONTAINER_HDR_SZ, equiv_tbl_len);
 
 	/* add header length */
-	return size + CONTAINER_HDR_SZ;
+	return equiv_tbl_len + CONTAINER_HDR_SZ;
 }
 
 static void free_equiv_cpu_table(void)
@@ -793,13 +792,12 @@ static enum ucode_state __load_microcode_amd(u8 family, const u8 *data,
 					     size_t size)
 {
 	u8 *fw = (u8 *)data;
-	int offset;
+	size_t offset;
 
-	offset = install_equiv_cpu_table(data);
-	if (offset < 0) {
-		pr_err("failed to create equivalent cpu table\n");
+	offset = install_equiv_cpu_table(data, size);
+	if (!offset)
 		return UCODE_ERROR;
-	}
+
 	fw   += offset;
 	size -= offset;
 
@@ -897,10 +895,8 @@ static enum ucode_state request_microcode_amd(int cpu, struct device *device,
 	}
 
 	ret = UCODE_ERROR;
-	if (*(u32 *)fw->data != UCODE_MAGIC) {
-		pr_err("invalid magic value (0x%08x)\n", *(u32 *)fw->data);
+	if (!verify_container(fw->data, fw->size, false))
 		goto fw_release;
-	}
 
 	ret = load_microcode_amd(bsp, c->x86, fw->data, fw->size);
 
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Check microcode container data in the late loader

[tip-bot for Maciej S. Szmigiero]

Commit-ID:  38673f623dfcaa03921aa5704ade176cd584c886
Gitweb:     https://git.kernel.org/tip/38673f623dfcaa03921aa5704ade176cd584c886
Author:     Maciej S. Szmigiero <mail@maciej.szmigiero.name>
AuthorDate: Thu, 13 Sep 2018 08:49:22 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:55:11 +0100

x86/microcode/AMD: Check microcode container data in the late loader

Convert the late loading path to use the newly introduced microcode
container data checking functions as it was previously done for the
early loader.

[ bp: Keep header length addition in install_equiv_cpu_table() and rediff. ]

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-14-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 38 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 66490ff087d9..ce6f8381641b 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -706,28 +706,27 @@ out:
 	return ret;
 }
 
-static int install_equiv_cpu_table(const u8 *buf)
+static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 {
-	unsigned int *ibuf = (unsigned int *)buf;
-	unsigned int type = ibuf[1];
-	unsigned int size = ibuf[2];
+	u32 equiv_tbl_len;
+	const u32 *hdr;
 
-	if (type != UCODE_EQUIV_CPU_TABLE_TYPE || !size) {
-		pr_err("empty section/"
-		       "invalid type field in container file section header\n");
-		return -EINVAL;
-	}
+	if (!verify_equivalence_table(buf, buf_size, false))
+		return 0;
 
-	equiv_cpu_table = vmalloc(size);
+	hdr = (const u32 *)buf;
+	equiv_tbl_len = hdr[2];
+
+	equiv_cpu_table = vmalloc(equiv_tbl_len);
 	if (!equiv_cpu_table) {
 		pr_err("failed to allocate equivalent CPU table\n");
-		return -ENOMEM;
+		return 0;
 	}
 
-	memcpy(equiv_cpu_table, buf + CONTAINER_HDR_SZ, size);
+	memcpy(equiv_cpu_table, buf + CONTAINER_HDR_SZ, equiv_tbl_len);
 
 	/* add header length */
-	return size + CONTAINER_HDR_SZ;
+	return equiv_tbl_len + CONTAINER_HDR_SZ;
 }
 
 static void free_equiv_cpu_table(void)
@@ -794,13 +793,12 @@ static enum ucode_state __load_microcode_amd(u8 family, const u8 *data,
 					     size_t size)
 {
 	u8 *fw = (u8 *)data;
-	int offset;
+	size_t offset;
 
-	offset = install_equiv_cpu_table(data);
-	if (offset < 0) {
-		pr_err("failed to create equivalent cpu table\n");
+	offset = install_equiv_cpu_table(data, size);
+	if (!offset)
 		return UCODE_ERROR;
-	}
+
 	fw   += offset;
 	size -= offset;
 
@@ -898,10 +896,8 @@ static enum ucode_state request_microcode_amd(int cpu, struct device *device,
 	}
 
 	ret = UCODE_ERROR;
-	if (*(u32 *)fw->data != UCODE_MAGIC) {
-		pr_err("invalid magic value (0x%08x)\n", *(u32 *)fw->data);
+	if (!verify_container(fw->data, fw->size, false))
 		goto fw_release;
-	}
 
 	ret = load_microcode_amd(bsp, c->x86, fw->data, fw->size);
 

[PATCH 14/16] x86/microcode/AMD: Convert CPU equivalence table variable into a struct

[Borislav Petkov]

From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>

Convert the CPU equivalence table into a proper struct in preparation
for tracking also the size of this table.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
[ Have functions deal with struct equiv_cpu_table pointers only. Rediff. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 32 ++++++++++++++++-------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 5d822ed295b5..99c9928ec240 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -38,7 +38,9 @@
 #include <asm/cpu.h>
 #include <asm/msr.h>
 
-static struct equiv_cpu_entry *equiv_cpu_table;
+static struct equiv_cpu_table {
+	struct equiv_cpu_entry *entry;
+} equiv_table;
 
 /*
  * This points to the current valid container of microcode patches which we will
@@ -63,11 +65,13 @@ 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(struct equiv_cpu_table *et, u32 sig)
 {
-	for (; equiv_table && equiv_table->installed_cpu; equiv_table++) {
-		if (sig == equiv_table->installed_cpu)
-			return equiv_table->equiv_cpu;
+	struct equiv_cpu_entry *entry = et->entry;
+
+	for (; entry && entry->installed_cpu; entry++) {
+		if (sig == entry->installed_cpu)
+			return entry->equiv_cpu;
 	}
 
 	return 0;
@@ -286,7 +290,7 @@ verify_patch(u8 family, const u8 *buf, size_t buf_size, u32 *patch_size, bool ea
  */
 static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
 {
-	struct equiv_cpu_entry *eq;
+	struct equiv_cpu_table table;
 	size_t orig_size = size;
 	u32 *hdr = (u32 *)ucode;
 	u16 eq_id;
@@ -297,14 +301,14 @@ static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
 
 	buf = ucode;
 
-	eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
+	table.entry = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
 	/*
 	 * Find the equivalence ID of our CPU in this table. Even if this table
 	 * doesn't contain a patch for the CPU, scan through the whole container
 	 * so that it can be skipped in case there are other containers appended.
 	 */
-	eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
+	eq_id = find_equiv_id(&table, desc->cpuid_1_eax);
 
 	buf  += hdr[2] + CONTAINER_HDR_SZ;
 	size -= hdr[2] + CONTAINER_HDR_SZ;
@@ -572,7 +576,7 @@ 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_table, uci->cpu_sig.sig);
 }
 
 /*
@@ -716,13 +720,13 @@ static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 	hdr = (const u32 *)buf;
 	equiv_tbl_len = hdr[2];
 
-	equiv_cpu_table = vmalloc(equiv_tbl_len);
-	if (!equiv_cpu_table) {
+	equiv_table.entry = vmalloc(equiv_tbl_len);
+	if (!equiv_table.entry) {
 		pr_err("failed to allocate equivalent CPU table\n");
 		return 0;
 	}
 
-	memcpy(equiv_cpu_table, buf + CONTAINER_HDR_SZ, equiv_tbl_len);
+	memcpy(equiv_table.entry, buf + CONTAINER_HDR_SZ, equiv_tbl_len);
 
 	/* add header length */
 	return equiv_tbl_len + CONTAINER_HDR_SZ;
@@ -730,8 +734,8 @@ static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 
 static void free_equiv_cpu_table(void)
 {
-	vfree(equiv_cpu_table);
-	equiv_cpu_table = NULL;
+	vfree(equiv_table.entry);
+	equiv_table.entry = NULL;
 }
 
 static void cleanup(void)
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Convert CPU equivalence table variable into a struct

[tip-bot for Maciej S. Szmigiero]

Commit-ID:  39cd7c17f9bc3fe3737dacd4225eeabe56df197c
Gitweb:     https://git.kernel.org/tip/39cd7c17f9bc3fe3737dacd4225eeabe56df197c
Author:     Maciej S. Szmigiero <mail@maciej.szmigiero.name>
AuthorDate: Thu, 13 Sep 2018 11:45:42 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:55:12 +0100

x86/microcode/AMD: Convert CPU equivalence table variable into a struct

Convert the CPU equivalence table into a proper struct in preparation
for tracking also the size of this table.

 [ bp: Have functions deal with struct equiv_cpu_table pointers only. Rediff. ]

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-15-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index ce6f8381641b..6048d9dd1a15 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -38,7 +38,9 @@
 #include <asm/cpu.h>
 #include <asm/msr.h>
 
-static struct equiv_cpu_entry *equiv_cpu_table;
+static struct equiv_cpu_table {
+	struct equiv_cpu_entry *entry;
+} equiv_table;
 
 /*
  * This points to the current valid container of microcode patches which we will
@@ -63,11 +65,13 @@ 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(struct equiv_cpu_table *et, u32 sig)
 {
-	for (; equiv_table && equiv_table->installed_cpu; equiv_table++) {
-		if (sig == equiv_table->installed_cpu)
-			return equiv_table->equiv_cpu;
+	struct equiv_cpu_entry *entry = et->entry;
+
+	for (; entry && entry->installed_cpu; entry++) {
+		if (sig == entry->installed_cpu)
+			return entry->equiv_cpu;
 	}
 
 	return 0;
@@ -286,7 +290,7 @@ verify_patch(u8 family, const u8 *buf, size_t buf_size, u32 *patch_size, bool ea
  */
 static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
 {
-	struct equiv_cpu_entry *eq;
+	struct equiv_cpu_table table;
 	size_t orig_size = size;
 	u32 *hdr = (u32 *)ucode;
 	u16 eq_id;
@@ -297,14 +301,14 @@ static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
 
 	buf = ucode;
 
-	eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
+	table.entry = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
 	/*
 	 * Find the equivalence ID of our CPU in this table. Even if this table
 	 * doesn't contain a patch for the CPU, scan through the whole container
 	 * so that it can be skipped in case there are other containers appended.
 	 */
-	eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
+	eq_id = find_equiv_id(&table, desc->cpuid_1_eax);
 
 	buf  += hdr[2] + CONTAINER_HDR_SZ;
 	size -= hdr[2] + CONTAINER_HDR_SZ;
@@ -573,7 +577,7 @@ 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_table, uci->cpu_sig.sig);
 }
 
 /*
@@ -717,13 +721,13 @@ static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 	hdr = (const u32 *)buf;
 	equiv_tbl_len = hdr[2];
 
-	equiv_cpu_table = vmalloc(equiv_tbl_len);
-	if (!equiv_cpu_table) {
+	equiv_table.entry = vmalloc(equiv_tbl_len);
+	if (!equiv_table.entry) {
 		pr_err("failed to allocate equivalent CPU table\n");
 		return 0;
 	}
 
-	memcpy(equiv_cpu_table, buf + CONTAINER_HDR_SZ, equiv_tbl_len);
+	memcpy(equiv_table.entry, buf + CONTAINER_HDR_SZ, equiv_tbl_len);
 
 	/* add header length */
 	return equiv_tbl_len + CONTAINER_HDR_SZ;
@@ -731,8 +735,8 @@ static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 
 static void free_equiv_cpu_table(void)
 {
-	vfree(equiv_cpu_table);
-	equiv_cpu_table = NULL;
+	vfree(equiv_table.entry);
+	equiv_table.entry = NULL;
 }
 
 static void cleanup(void)

[PATCH 15/16] x86/microcode/AMD: Check the equivalence table size when scanning it

[Borislav Petkov]

From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>

Currently, the code scanning the CPU equivalence table read from a
microcode container file assumes that it actually contains a terminating
zero entry.

Check also the size of this table to make sure that no reads past its
end happen, in case there's no terminating zero entry at the end of the
table.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
[ Adjust to new changes. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 99c9928ec240..dc17a5f87f55 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_table {
+	unsigned int num_entries;
 	struct equiv_cpu_entry *entry;
 } equiv_table;
 
@@ -67,13 +68,19 @@ ucode_path[] __maybe_unused = "kernel/x86/microcode/AuthenticAMD.bin";
 
 static u16 find_equiv_id(struct equiv_cpu_table *et, u32 sig)
 {
-	struct equiv_cpu_entry *entry = et->entry;
+	unsigned int i;
 
-	for (; entry && entry->installed_cpu; entry++) {
-		if (sig == entry->installed_cpu)
-			return entry->equiv_cpu;
-	}
+	if (!et || !et->num_entries)
+		return 0;
+
+	for (i = 0; i < et->num_entries; i++) {
+		struct equiv_cpu_entry *e = &et->entry[i];
 
+		if (sig == e->installed_cpu)
+			return e->equiv_cpu;
+
+		e++;
+	}
 	return 0;
 }
 
@@ -302,6 +309,7 @@ static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
 	buf = ucode;
 
 	table.entry = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
+	table.num_entries = hdr[2] / sizeof(struct equiv_cpu_entry);
 
 	/*
 	 * Find the equivalence ID of our CPU in this table. Even if this table
@@ -727,6 +735,7 @@ static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 	}
 
 	memcpy(equiv_table.entry, buf + CONTAINER_HDR_SZ, equiv_tbl_len);
+	equiv_table.num_entries = equiv_tbl_len / sizeof(struct equiv_cpu_entry);
 
 	/* add header length */
 	return equiv_tbl_len + CONTAINER_HDR_SZ;
@@ -735,7 +744,7 @@ static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 static void free_equiv_cpu_table(void)
 {
 	vfree(equiv_table.entry);
-	equiv_table.entry = NULL;
+	memset(&equiv_table, 0, sizeof(equiv_table));
 }
 
 static void cleanup(void)
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Check the equivalence table size when scanning it

[tip-bot for Maciej S. Szmigiero]

Commit-ID:  413c89154c6759cbd5e17febd04c187470613173
Gitweb:     https://git.kernel.org/tip/413c89154c6759cbd5e17febd04c187470613173
Author:     Maciej S. Szmigiero <mail@maciej.szmigiero.name>
AuthorDate: Thu, 13 Sep 2018 12:01:52 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:55:12 +0100

x86/microcode/AMD: Check the equivalence table size when scanning it

Currently, the code scanning the CPU equivalence table read from a
microcode container file assumes that it actually contains a terminating
zero entry.

Check also the size of this table to make sure that no reads past its
end happen, in case there's no terminating zero entry at the end of the
table.

 [ bp: Adjust to new changes. ]

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-16-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 6048d9dd1a15..81df4b9eadb7 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_table {
+	unsigned int num_entries;
 	struct equiv_cpu_entry *entry;
 } equiv_table;
 
@@ -67,13 +68,19 @@ ucode_path[] __maybe_unused = "kernel/x86/microcode/AuthenticAMD.bin";
 
 static u16 find_equiv_id(struct equiv_cpu_table *et, u32 sig)
 {
-	struct equiv_cpu_entry *entry = et->entry;
+	unsigned int i;
 
-	for (; entry && entry->installed_cpu; entry++) {
-		if (sig == entry->installed_cpu)
-			return entry->equiv_cpu;
-	}
+	if (!et || !et->num_entries)
+		return 0;
+
+	for (i = 0; i < et->num_entries; i++) {
+		struct equiv_cpu_entry *e = &et->entry[i];
 
+		if (sig == e->installed_cpu)
+			return e->equiv_cpu;
+
+		e++;
+	}
 	return 0;
 }
 
@@ -302,6 +309,7 @@ static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
 	buf = ucode;
 
 	table.entry = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
+	table.num_entries = hdr[2] / sizeof(struct equiv_cpu_entry);
 
 	/*
 	 * Find the equivalence ID of our CPU in this table. Even if this table
@@ -728,6 +736,7 @@ static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 	}
 
 	memcpy(equiv_table.entry, buf + CONTAINER_HDR_SZ, equiv_tbl_len);
+	equiv_table.num_entries = equiv_tbl_len / sizeof(struct equiv_cpu_entry);
 
 	/* add header length */
 	return equiv_tbl_len + CONTAINER_HDR_SZ;
@@ -736,7 +745,7 @@ static size_t install_equiv_cpu_table(const u8 *buf, size_t buf_size)
 static void free_equiv_cpu_table(void)
 {
 	vfree(equiv_table.entry);
-	equiv_table.entry = NULL;
+	memset(&equiv_table, 0, sizeof(equiv_table));
 }
 
 static void cleanup(void)

[PATCH 16/16] x86/microcode/AMD: Update copyright

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Adjust copyright.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index dc17a5f87f55..ba5deed576d1 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -5,7 +5,7 @@
  *  CPUs and later.
  *
  *  Copyright (C) 2008-2011 Advanced Micro Devices Inc.
- *	          2013-2016 Borislav Petkov <bp@alien8.de>
+ *	          2013-2018 Borislav Petkov <bp@alien8.de>
  *
  *  Author: Peter Oruba <peter.oruba@amd.com>
  *
-- 
2.19.1

[tip:x86/microcode] x86/microcode/AMD: Update copyright

[tip-bot for Borislav Petkov]

Commit-ID:  2ffcbce39ea1e355eed8c6527308624f366f1c58
Gitweb:     https://git.kernel.org/tip/2ffcbce39ea1e355eed8c6527308624f366f1c58
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Sat, 20 Oct 2018 15:04:52 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 19 Nov 2018 10:55:12 +0100

x86/microcode/AMD: Update copyright

Adjust copyright.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
Link: https://lkml.kernel.org/r/20181107170218.7596-17-bp@alien8.de
---
 arch/x86/kernel/cpu/microcode/amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 81df4b9eadb7..51adde0a0f1a 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -5,7 +5,7 @@
  *  CPUs and later.
  *
  *  Copyright (C) 2008-2011 Advanced Micro Devices Inc.
- *	          2013-2016 Borislav Petkov <bp@alien8.de>
+ *	          2013-2018 Borislav Petkov <bp@alien8.de>
  *
  *  Author: Peter Oruba <peter.oruba@amd.com>
  *

Re: [PATCH 00/16] x86/microcode/AMD: Improve container verification

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]

On Wed 2018-11-07 18:02:02, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Hi all,
> 
> this is work which got started by Maciej a while ago. I have finally had
> the time to redo all the ideas properly, split it in self-contained,
> logical chunks and test it.
> 
> Reveiew and comments are appreciated.

Unfortunately, this is not too useful cover letter. It does not tell
us what the goals if the series are, for example :-(.
								Pavel

> Thx.
> 
> Borislav Petkov (11):
>   x86/microcode/AMD: Move verify_patch_size() up in the file
>   x86/microcode/AMD: Clean up per-family patch size checks
>   x86/microcode/AMD: Cleanup verify_patch_size() more
>   x86/microcode/AMD: Concentrate patch verification
>   x86/microcode/AMD: Simplify patch family detection
>   x86/microcode/AMD: Move patch family check to verify_patch()
>   x86/microcode/AMD: Move chipset-specific check into verify_patch()
>   x86/microcode/AMD: Change verify_patch()'s return value
>   x86/microcode/AMD: Convert early parser to the new verification routines
>   x86/microcode/AMD: Fix container size's type
>   x86/microcode/AMD: Update copyright
> 
> Maciej S. Szmigiero (5):
>   x86/microcode/AMD: Subtract SECTION_HDR_SIZE from file leftover length
>   x86/microcode/AMD: Add microcode container verification
>   x86/microcode/AMD: Check microcode container data in the late loader
>   x86/microcode/AMD: Convert CPU equivalence table variable into a struct
>   x86/microcode/AMD: Check the equivalence table size when scanning it
> 
>  arch/x86/kernel/cpu/microcode/amd.c | 469 ++++++++++++++++++----------
>  1 file changed, 307 insertions(+), 162 deletions(-)
> 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]