linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] x86/microcode/intel: Microcode verification cleanups
@ 2016-03-07 10:09 Borislav Petkov
  2016-03-07 10:10 ` [PATCH 1/5] x86/microcode/intel: Change checksum variables to u32 Borislav Petkov
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Borislav Petkov @ 2016-03-07 10:09 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: X86 ML, LKML

From: Borislav Petkov <bp@suse.de>

Hi,

here's a small set which makes sure the microcode data checksum
verification is done using u32s, as the SDM says and not what we did.
The interesting part is that Chris Bainbridge caught this with UBSAN
even though the SDM has been proclaiming using unsigned dwords already.

The rest are some trivial cleanups which sprang out after me staring at
microcode_sanity_check(). FWIW, it could use some more but that's for
later.

Patches ontop of tip/x86/microcode.

Borislav Petkov (4):
  x86/microcode/intel: Get rid of DWSIZE
  x86/microcode/intel: Merge two consecutive if-statements
  x86/microcode/intel: Improve microcode sanity-checking error messages
  x86/microcode/intel: Drop orig_sum from ext signature checksum

Chris Bainbridge (1):
  x86/microcode/intel: Change checksum variables to u32

 arch/x86/include/asm/microcode_intel.h    |  1 -
 arch/x86/kernel/cpu/microcode/intel_lib.c | 58 ++++++++++++++++++++-----------
 2 files changed, 37 insertions(+), 22 deletions(-)

-- 
2.3.5

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/5] x86/microcode/intel: Change checksum variables to u32
  2016-03-07 10:09 [PATCH 0/5] x86/microcode/intel: Microcode verification cleanups Borislav Petkov
@ 2016-03-07 10:10 ` Borislav Petkov
  2016-03-07 10:10 ` [PATCH 2/5] x86/microcode/intel: Get rid of DWSIZE Borislav Petkov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2016-03-07 10:10 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: X86 ML, LKML, H. Peter Anvin, Ingo Molnar, Thomas Gleixner, hmh

From: Chris Bainbridge <chris.bainbridge@gmail.com>

Microcode checksum verification should be done using unsigned 32-bit
values otherwise the calculation overflow results in undefined
behaviour.

This is also nicely documented in the SDM, section "Microcode Update
Checksum":

  "To check for a corrupt microcode update, software must perform a
  unsigned DWORD (32-bit) checksum of the microcode update. Even though
  some fields are signed, the checksum procedure treats all DWORDs as
  unsigned. Microcode updates with a header version equal to 00000001H
  must sum all DWORDs that comprise the microcode update. A valid
  checksum check will yield a value of 00000000H."

but for some reason the code has been using ints from the very
beginning.

In practice, this bug possibly manifested itself only when doing the
microcode data checksum - apparently, currently shipped Intel microcode
doesn't have an extended signature table for which we do checksum
verification too.

  UBSAN: Undefined behaviour in arch/x86/kernel/cpu/microcode/intel_lib.c:105:12
  signed integer overflow:
  -1500151068 + -2125470173 cannot be represented in type 'int'
  CPU: 0 PID: 0 Comm: swapper Not tainted 4.5.0-rc5+ #495
  ...
  Call Trace:
   dump_stack
   ? inotify_ioctl
   ubsan_epilogue
   handle_overflow
   __ubsan_handle_add_overflow
   microcode_sanity_check
   get_matching_model_microcode.isra.2.constprop.8
   ? early_idt_handler_common
   ? strlcpy
   ? find_cpio_data
   load_ucode_intel_bsp
   load_ucode_bsp
   ? load_ucode_bsp
   x86_64_start_kernel

Signed-off-by: Chris Bainbridge <chris.bainbridge@gmail.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: hmh@hmh.eng.br
Cc: x86-ml <x86@kernel.org>
Link: http://lkml.kernel.org/r/1456834359-5132-1-git-send-email-chris.bainbridge@gmail.com
[ Expand and massage commit message. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/microcode/intel_lib.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c
index b96896bcbdaf..99ca2c935777 100644
--- a/arch/x86/kernel/cpu/microcode/intel_lib.c
+++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
@@ -49,7 +49,7 @@ int microcode_sanity_check(void *mc, int print_err)
 	unsigned long total_size, data_size, ext_table_size;
 	struct microcode_header_intel *mc_header = mc;
 	struct extended_sigtable *ext_header = NULL;
-	int sum, orig_sum, ext_sigcount = 0, i;
+	u32 sum, orig_sum, ext_sigcount = 0, i;
 	struct extended_signature *ext_sig;
 
 	total_size = get_totalsize(mc_header);
@@ -85,8 +85,8 @@ int microcode_sanity_check(void *mc, int print_err)
 
 	/* check extended table checksum */
 	if (ext_table_size) {
-		int ext_table_sum = 0;
-		int *ext_tablep = (int *)ext_header;
+		u32 ext_table_sum = 0;
+		u32 *ext_tablep = (u32 *)ext_header;
 
 		i = ext_table_size / DWSIZE;
 		while (i--)
@@ -102,7 +102,7 @@ int microcode_sanity_check(void *mc, int print_err)
 	orig_sum = 0;
 	i = (MC_HEADER_SIZE + data_size) / DWSIZE;
 	while (i--)
-		orig_sum += ((int *)mc)[i];
+		orig_sum += ((u32 *)mc)[i];
 	if (orig_sum) {
 		if (print_err)
 			pr_err("aborting, bad checksum\n");
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/5] x86/microcode/intel: Get rid of DWSIZE
  2016-03-07 10:09 [PATCH 0/5] x86/microcode/intel: Microcode verification cleanups Borislav Petkov
  2016-03-07 10:10 ` [PATCH 1/5] x86/microcode/intel: Change checksum variables to u32 Borislav Petkov
@ 2016-03-07 10:10 ` Borislav Petkov
  2016-03-08  8:12   ` [tip:x86/microcode] " tip-bot for Borislav Petkov
  2016-03-07 10:10 ` [PATCH 3/5] x86/microcode/intel: Merge two consecutive if-statements Borislav Petkov
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2016-03-07 10:10 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: X86 ML, LKML

From: Borislav Petkov <bp@suse.de>

sizeof(u32) is perfectly clear as it is.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/microcode_intel.h    | 1 -
 arch/x86/kernel/cpu/microcode/intel_lib.c | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 8559b0102ea1..603417f8dd6c 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -40,7 +40,6 @@ struct extended_sigtable {
 #define DEFAULT_UCODE_TOTALSIZE (DEFAULT_UCODE_DATASIZE + MC_HEADER_SIZE)
 #define EXT_HEADER_SIZE		(sizeof(struct extended_sigtable))
 #define EXT_SIGNATURE_SIZE	(sizeof(struct extended_signature))
-#define DWSIZE			(sizeof(u32))
 
 #define get_totalsize(mc) \
 	(((struct microcode_intel *)mc)->hdr.datasize ? \
diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c
index 99ca2c935777..2b2d1354dc70 100644
--- a/arch/x86/kernel/cpu/microcode/intel_lib.c
+++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
@@ -88,7 +88,7 @@ int microcode_sanity_check(void *mc, int print_err)
 		u32 ext_table_sum = 0;
 		u32 *ext_tablep = (u32 *)ext_header;
 
-		i = ext_table_size / DWSIZE;
+		i = ext_table_size / sizeof(u32);
 		while (i--)
 			ext_table_sum += ext_tablep[i];
 		if (ext_table_sum) {
@@ -100,7 +100,7 @@ int microcode_sanity_check(void *mc, int print_err)
 
 	/* calculate the checksum */
 	orig_sum = 0;
-	i = (MC_HEADER_SIZE + data_size) / DWSIZE;
+	i = (MC_HEADER_SIZE + data_size) / sizeof(u32);
 	while (i--)
 		orig_sum += ((u32 *)mc)[i];
 	if (orig_sum) {
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/5] x86/microcode/intel: Merge two consecutive if-statements
  2016-03-07 10:09 [PATCH 0/5] x86/microcode/intel: Microcode verification cleanups Borislav Petkov
  2016-03-07 10:10 ` [PATCH 1/5] x86/microcode/intel: Change checksum variables to u32 Borislav Petkov
  2016-03-07 10:10 ` [PATCH 2/5] x86/microcode/intel: Get rid of DWSIZE Borislav Petkov
@ 2016-03-07 10:10 ` Borislav Petkov
  2016-03-08  8:13   ` [tip:x86/microcode] " tip-bot for Borislav Petkov
  2016-03-07 10:10 ` [PATCH 4/5] x86/microcode/intel: Improve microcode sanity-checking error messages Borislav Petkov
  2016-03-07 10:10 ` [PATCH 5/5] x86/microcode/intel: Drop orig_sum from ext signature checksum Borislav Petkov
  4 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2016-03-07 10:10 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: X86 ML, LKML

From: Borislav Petkov <bp@suse.de>

Merge the two consecutive "if (ext_table_size)".

No functionality change.

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

diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c
index 2b2d1354dc70..ffb1bbf45db0 100644
--- a/arch/x86/kernel/cpu/microcode/intel_lib.c
+++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
@@ -68,6 +68,9 @@ int microcode_sanity_check(void *mc, int print_err)
 	}
 	ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
 	if (ext_table_size) {
+		u32 ext_table_sum = 0;
+		u32 *ext_tablep;
+
 		if ((ext_table_size < EXT_HEADER_SIZE)
 		 || ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE)) {
 			if (print_err)
@@ -81,12 +84,9 @@ int microcode_sanity_check(void *mc, int print_err)
 			return -EFAULT;
 		}
 		ext_sigcount = ext_header->count;
-	}
 
-	/* check extended table checksum */
-	if (ext_table_size) {
-		u32 ext_table_sum = 0;
-		u32 *ext_tablep = (u32 *)ext_header;
+		/* check extended table checksum */
+		ext_tablep = (u32 *)ext_header;
 
 		i = ext_table_size / sizeof(u32);
 		while (i--)
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/5] x86/microcode/intel: Improve microcode sanity-checking error messages
  2016-03-07 10:09 [PATCH 0/5] x86/microcode/intel: Microcode verification cleanups Borislav Petkov
                   ` (2 preceding siblings ...)
  2016-03-07 10:10 ` [PATCH 3/5] x86/microcode/intel: Merge two consecutive if-statements Borislav Petkov
@ 2016-03-07 10:10 ` Borislav Petkov
  2016-03-08  8:13   ` [tip:x86/microcode] " tip-bot for Borislav Petkov
  2016-03-07 10:10 ` [PATCH 5/5] x86/microcode/intel: Drop orig_sum from ext signature checksum Borislav Petkov
  4 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2016-03-07 10:10 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: X86 ML, LKML

From: Borislav Petkov <bp@suse.de>

Turn them into proper sentences. Add comments to
microcode_sanity_check() to explain what it does.

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

diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c
index ffb1bbf45db0..23b1d92342e3 100644
--- a/arch/x86/kernel/cpu/microcode/intel_lib.c
+++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
@@ -57,15 +57,16 @@ int microcode_sanity_check(void *mc, int print_err)
 
 	if (data_size + MC_HEADER_SIZE > total_size) {
 		if (print_err)
-			pr_err("error! Bad data size in microcode data file\n");
+			pr_err("Error: bad microcode data file size.\n");
 		return -EINVAL;
 	}
 
 	if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
 		if (print_err)
-			pr_err("error! Unknown microcode update format\n");
+			pr_err("Error: invalid/unknown microcode update format.\n");
 		return -EINVAL;
 	}
+
 	ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
 	if (ext_table_size) {
 		u32 ext_table_sum = 0;
@@ -74,43 +75,58 @@ int microcode_sanity_check(void *mc, int print_err)
 		if ((ext_table_size < EXT_HEADER_SIZE)
 		 || ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE)) {
 			if (print_err)
-				pr_err("error! Small exttable size in microcode data file\n");
+				pr_err("Error: truncated extended signature table.\n");
 			return -EINVAL;
 		}
+
 		ext_header = mc + MC_HEADER_SIZE + data_size;
 		if (ext_table_size != exttable_size(ext_header)) {
 			if (print_err)
-				pr_err("error! Bad exttable size in microcode data file\n");
+				pr_err("Error: extended signature table size mismatch.\n");
 			return -EFAULT;
 		}
+
 		ext_sigcount = ext_header->count;
 
-		/* check extended table checksum */
+		/*
+		 * Check extended table checksum: the sum of all dwords that
+		 * comprise a valid table must be 0.
+		 */
 		ext_tablep = (u32 *)ext_header;
 
 		i = ext_table_size / sizeof(u32);
 		while (i--)
 			ext_table_sum += ext_tablep[i];
+
 		if (ext_table_sum) {
 			if (print_err)
-				pr_warn("aborting, bad extended signature table checksum\n");
+				pr_warn("Bad extended signature table checksum, aborting.\n");
 			return -EINVAL;
 		}
 	}
 
-	/* calculate the checksum */
+	/*
+	 * Calculate the checksum of update data and header. The checksum of
+	 * valid update data and header including the extended signature table
+	 * must be 0.
+	 */
 	orig_sum = 0;
 	i = (MC_HEADER_SIZE + data_size) / sizeof(u32);
 	while (i--)
 		orig_sum += ((u32 *)mc)[i];
+
 	if (orig_sum) {
 		if (print_err)
-			pr_err("aborting, bad checksum\n");
+			pr_err("Bad microcode data checksum, aborting.\n");
 		return -EINVAL;
 	}
+
 	if (!ext_table_size)
 		return 0;
-	/* check extended signature checksum */
+
+	/*
+	 * Check extended signature checksum: 0 => valid.
+	 */
 	for (i = 0; i < ext_sigcount; i++) {
 		ext_sig = (void *)ext_header + EXT_HEADER_SIZE +
 			  EXT_SIGNATURE_SIZE * i;
@@ -119,7 +135,7 @@ int microcode_sanity_check(void *mc, int print_err)
 			+ (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
 		if (sum) {
 			if (print_err)
-				pr_err("aborting, bad checksum\n");
+				pr_err("Bad extended signature checksum, aborting.\n");
 			return -EINVAL;
 		}
 	}
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/5] x86/microcode/intel: Drop orig_sum from ext signature checksum
  2016-03-07 10:09 [PATCH 0/5] x86/microcode/intel: Microcode verification cleanups Borislav Petkov
                   ` (3 preceding siblings ...)
  2016-03-07 10:10 ` [PATCH 4/5] x86/microcode/intel: Improve microcode sanity-checking error messages Borislav Petkov
@ 2016-03-07 10:10 ` Borislav Petkov
  2016-03-08  8:14   ` [tip:x86/microcode] " tip-bot for Borislav Petkov
  4 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2016-03-07 10:10 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: X86 ML, LKML

From: Borislav Petkov <bp@suse.de>

It is 0 because for !0 values we would have exited already.

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

diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c
index 23b1d92342e3..2ce1a7dc45b7 100644
--- a/arch/x86/kernel/cpu/microcode/intel_lib.c
+++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
@@ -130,9 +130,9 @@ int microcode_sanity_check(void *mc, int print_err)
 	for (i = 0; i < ext_sigcount; i++) {
 		ext_sig = (void *)ext_header + EXT_HEADER_SIZE +
 			  EXT_SIGNATURE_SIZE * i;
-		sum = orig_sum
-			- (mc_header->sig + mc_header->pf + mc_header->cksum)
-			+ (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
+
+		sum = (mc_header->sig + mc_header->pf + mc_header->cksum) -
+		      (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
 		if (sum) {
 			if (print_err)
 				pr_err("Bad extended signature checksum, aborting.\n");
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:x86/microcode] x86/microcode/intel: Get rid of DWSIZE
  2016-03-07 10:10 ` [PATCH 2/5] x86/microcode/intel: Get rid of DWSIZE Borislav Petkov
@ 2016-03-08  8:12   ` tip-bot for Borislav Petkov
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Borislav Petkov @ 2016-03-08  8:12 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: hpa, bp, linux-kernel, mingo, tglx

Commit-ID:  c0414622177ae4739a49ca7dad4306a681e2878b
Gitweb:     http://git.kernel.org/tip/c0414622177ae4739a49ca7dad4306a681e2878b
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Mon, 7 Mar 2016 11:10:01 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 8 Mar 2016 09:08:44 +0100

x86/microcode/intel: Get rid of DWSIZE

sizeof(u32) is perfectly clear as it is.

Signed-off-by: Borislav Petkov <bp@suse.de>
Link: http://lkml.kernel.org/r/1457345404-28884-3-git-send-email-bp@alien8.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/microcode_intel.h    | 1 -
 arch/x86/kernel/cpu/microcode/intel_lib.c | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 8559b01..603417f 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -40,7 +40,6 @@ struct extended_sigtable {
 #define DEFAULT_UCODE_TOTALSIZE (DEFAULT_UCODE_DATASIZE + MC_HEADER_SIZE)
 #define EXT_HEADER_SIZE		(sizeof(struct extended_sigtable))
 #define EXT_SIGNATURE_SIZE	(sizeof(struct extended_signature))
-#define DWSIZE			(sizeof(u32))
 
 #define get_totalsize(mc) \
 	(((struct microcode_intel *)mc)->hdr.datasize ? \
diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c
index 99ca2c9..2b2d135 100644
--- a/arch/x86/kernel/cpu/microcode/intel_lib.c
+++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
@@ -88,7 +88,7 @@ int microcode_sanity_check(void *mc, int print_err)
 		u32 ext_table_sum = 0;
 		u32 *ext_tablep = (u32 *)ext_header;
 
-		i = ext_table_size / DWSIZE;
+		i = ext_table_size / sizeof(u32);
 		while (i--)
 			ext_table_sum += ext_tablep[i];
 		if (ext_table_sum) {
@@ -100,7 +100,7 @@ int microcode_sanity_check(void *mc, int print_err)
 
 	/* calculate the checksum */
 	orig_sum = 0;
-	i = (MC_HEADER_SIZE + data_size) / DWSIZE;
+	i = (MC_HEADER_SIZE + data_size) / sizeof(u32);
 	while (i--)
 		orig_sum += ((u32 *)mc)[i];
 	if (orig_sum) {

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:x86/microcode] x86/microcode/intel: Merge two consecutive if-statements
  2016-03-07 10:10 ` [PATCH 3/5] x86/microcode/intel: Merge two consecutive if-statements Borislav Petkov
@ 2016-03-08  8:13   ` tip-bot for Borislav Petkov
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Borislav Petkov @ 2016-03-08  8:13 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, mingo, tglx, hpa, bp

Commit-ID:  7d0161569a3b66aaa01520002c3e5fd7126d071f
Gitweb:     http://git.kernel.org/tip/7d0161569a3b66aaa01520002c3e5fd7126d071f
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Mon, 7 Mar 2016 11:10:02 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 8 Mar 2016 09:08:45 +0100

x86/microcode/intel: Merge two consecutive if-statements

Merge the two consecutive "if (ext_table_size)". No functional change.

Signed-off-by: Borislav Petkov <bp@suse.de>
Link: http://lkml.kernel.org/r/1457345404-28884-4-git-send-email-bp@alien8.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/cpu/microcode/intel_lib.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c
index 2b2d135..ffb1bbf 100644
--- a/arch/x86/kernel/cpu/microcode/intel_lib.c
+++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
@@ -68,6 +68,9 @@ int microcode_sanity_check(void *mc, int print_err)
 	}
 	ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
 	if (ext_table_size) {
+		u32 ext_table_sum = 0;
+		u32 *ext_tablep;
+
 		if ((ext_table_size < EXT_HEADER_SIZE)
 		 || ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE)) {
 			if (print_err)
@@ -81,12 +84,9 @@ int microcode_sanity_check(void *mc, int print_err)
 			return -EFAULT;
 		}
 		ext_sigcount = ext_header->count;
-	}
 
-	/* check extended table checksum */
-	if (ext_table_size) {
-		u32 ext_table_sum = 0;
-		u32 *ext_tablep = (u32 *)ext_header;
+		/* check extended table checksum */
+		ext_tablep = (u32 *)ext_header;
 
 		i = ext_table_size / sizeof(u32);
 		while (i--)

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:x86/microcode] x86/microcode/intel: Improve microcode sanity-checking error messages
  2016-03-07 10:10 ` [PATCH 4/5] x86/microcode/intel: Improve microcode sanity-checking error messages Borislav Petkov
@ 2016-03-08  8:13   ` tip-bot for Borislav Petkov
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Borislav Petkov @ 2016-03-08  8:13 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: bp, linux-kernel, hpa, mingo, tglx

Commit-ID:  5b46b5e003724547f0c83041cada15f9f496590d
Gitweb:     http://git.kernel.org/tip/5b46b5e003724547f0c83041cada15f9f496590d
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Mon, 7 Mar 2016 11:10:03 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 8 Mar 2016 09:08:45 +0100

x86/microcode/intel: Improve microcode sanity-checking error messages

Turn them into proper sentences. Add comments to microcode_sanity_check() to
explain what it does.

Signed-off-by: Borislav Petkov <bp@suse.de>
Link: http://lkml.kernel.org/r/1457345404-28884-5-git-send-email-bp@alien8.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/cpu/microcode/intel_lib.c | 36 ++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c
index ffb1bbf..23b1d92 100644
--- a/arch/x86/kernel/cpu/microcode/intel_lib.c
+++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
@@ -57,15 +57,16 @@ int microcode_sanity_check(void *mc, int print_err)
 
 	if (data_size + MC_HEADER_SIZE > total_size) {
 		if (print_err)
-			pr_err("error! Bad data size in microcode data file\n");
+			pr_err("Error: bad microcode data file size.\n");
 		return -EINVAL;
 	}
 
 	if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
 		if (print_err)
-			pr_err("error! Unknown microcode update format\n");
+			pr_err("Error: invalid/unknown microcode update format.\n");
 		return -EINVAL;
 	}
+
 	ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
 	if (ext_table_size) {
 		u32 ext_table_sum = 0;
@@ -74,43 +75,58 @@ int microcode_sanity_check(void *mc, int print_err)
 		if ((ext_table_size < EXT_HEADER_SIZE)
 		 || ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE)) {
 			if (print_err)
-				pr_err("error! Small exttable size in microcode data file\n");
+				pr_err("Error: truncated extended signature table.\n");
 			return -EINVAL;
 		}
+
 		ext_header = mc + MC_HEADER_SIZE + data_size;
 		if (ext_table_size != exttable_size(ext_header)) {
 			if (print_err)
-				pr_err("error! Bad exttable size in microcode data file\n");
+				pr_err("Error: extended signature table size mismatch.\n");
 			return -EFAULT;
 		}
+
 		ext_sigcount = ext_header->count;
 
-		/* check extended table checksum */
+		/*
+		 * Check extended table checksum: the sum of all dwords that
+		 * comprise a valid table must be 0.
+		 */
 		ext_tablep = (u32 *)ext_header;
 
 		i = ext_table_size / sizeof(u32);
 		while (i--)
 			ext_table_sum += ext_tablep[i];
+
 		if (ext_table_sum) {
 			if (print_err)
-				pr_warn("aborting, bad extended signature table checksum\n");
+				pr_warn("Bad extended signature table checksum, aborting.\n");
 			return -EINVAL;
 		}
 	}
 
-	/* calculate the checksum */
+	/*
+	 * Calculate the checksum of update data and header. The checksum of
+	 * valid update data and header including the extended signature table
+	 * must be 0.
+	 */
 	orig_sum = 0;
 	i = (MC_HEADER_SIZE + data_size) / sizeof(u32);
 	while (i--)
 		orig_sum += ((u32 *)mc)[i];
+
 	if (orig_sum) {
 		if (print_err)
-			pr_err("aborting, bad checksum\n");
+			pr_err("Bad microcode data checksum, aborting.\n");
 		return -EINVAL;
 	}
+
 	if (!ext_table_size)
 		return 0;
-	/* check extended signature checksum */
+
+	/*
+	 * Check extended signature checksum: 0 => valid.
+	 */
 	for (i = 0; i < ext_sigcount; i++) {
 		ext_sig = (void *)ext_header + EXT_HEADER_SIZE +
 			  EXT_SIGNATURE_SIZE * i;
@@ -119,7 +135,7 @@ int microcode_sanity_check(void *mc, int print_err)
 			+ (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
 		if (sum) {
 			if (print_err)
-				pr_err("aborting, bad checksum\n");
+				pr_err("Bad extended signature checksum, aborting.\n");
 			return -EINVAL;
 		}
 	}

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:x86/microcode] x86/microcode/intel: Drop orig_sum from ext signature checksum
  2016-03-07 10:10 ` [PATCH 5/5] x86/microcode/intel: Drop orig_sum from ext signature checksum Borislav Petkov
@ 2016-03-08  8:14   ` tip-bot for Borislav Petkov
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Borislav Petkov @ 2016-03-08  8:14 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, bp, tglx, hpa, mingo

Commit-ID:  4ace2e7a48ab426eaa9745ace4c50c6a7adb3992
Gitweb:     http://git.kernel.org/tip/4ace2e7a48ab426eaa9745ace4c50c6a7adb3992
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Mon, 7 Mar 2016 11:10:04 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 8 Mar 2016 09:08:45 +0100

x86/microcode/intel: Drop orig_sum from ext signature checksum

It is 0 because for !0 values we would have exited already.

Signed-off-by: Borislav Petkov <bp@suse.de>
Link: http://lkml.kernel.org/r/1457345404-28884-6-git-send-email-bp@alien8.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/cpu/microcode/intel_lib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c
index 23b1d92..2ce1a7d 100644
--- a/arch/x86/kernel/cpu/microcode/intel_lib.c
+++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
@@ -130,9 +130,9 @@ int microcode_sanity_check(void *mc, int print_err)
 	for (i = 0; i < ext_sigcount; i++) {
 		ext_sig = (void *)ext_header + EXT_HEADER_SIZE +
 			  EXT_SIGNATURE_SIZE * i;
-		sum = orig_sum
-			- (mc_header->sig + mc_header->pf + mc_header->cksum)
-			+ (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
+
+		sum = (mc_header->sig + mc_header->pf + mc_header->cksum) -
+		      (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
 		if (sum) {
 			if (print_err)
 				pr_err("Bad extended signature checksum, aborting.\n");

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-03-08  8:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-07 10:09 [PATCH 0/5] x86/microcode/intel: Microcode verification cleanups Borislav Petkov
2016-03-07 10:10 ` [PATCH 1/5] x86/microcode/intel: Change checksum variables to u32 Borislav Petkov
2016-03-07 10:10 ` [PATCH 2/5] x86/microcode/intel: Get rid of DWSIZE Borislav Petkov
2016-03-08  8:12   ` [tip:x86/microcode] " tip-bot for Borislav Petkov
2016-03-07 10:10 ` [PATCH 3/5] x86/microcode/intel: Merge two consecutive if-statements Borislav Petkov
2016-03-08  8:13   ` [tip:x86/microcode] " tip-bot for Borislav Petkov
2016-03-07 10:10 ` [PATCH 4/5] x86/microcode/intel: Improve microcode sanity-checking error messages Borislav Petkov
2016-03-08  8:13   ` [tip:x86/microcode] " tip-bot for Borislav Petkov
2016-03-07 10:10 ` [PATCH 5/5] x86/microcode/intel: Drop orig_sum from ext signature checksum Borislav Petkov
2016-03-08  8:14   ` [tip:x86/microcode] " tip-bot for Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).