mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Ingo Molnar <mingo@kernel.org>
Cc: X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	hmh@hmh.eng.br
Subject: [PATCH 1/5] x86/microcode/intel: Change checksum variables to u32
Date: Mon,  7 Mar 2016 11:10:00 +0100	[thread overview]
Message-ID: <1457345404-28884-2-git-send-email-bp@alien8.de> (raw)
In-Reply-To: <1457345404-28884-1-git-send-email-bp@alien8.de>

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

  reply	other threads:[~2016-03-07 10:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-07 10:09 [PATCH 0/5] x86/microcode/intel: Microcode verification cleanups Borislav Petkov
2016-03-07 10:10 ` Borislav Petkov [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1457345404-28884-2-git-send-email-bp@alien8.de \
    --to=bp@alien8.de \
    --cc=hmh@hmh.eng.br \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome