13901 Incorrect calculation of Intel microcode extended patch signature checksum

Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Approved by: Robert Mustacchi <rm@fingolfin.org>

5 files changed, 64 insertions, 34 deletions

diff --git a/usr/src/cmd/ucodeadm/ucodeadm.c b/usr/src/cmd/ucodeadm/ucodeadm.c
index 18d0162b82..65f61facea 100644
--- a/usr/src/cmd/ucodeadm/ucodeadm.c
+++ b/usr/src/cmd/ucodeadm/ucodeadm.c
@@ -387,11 +387,10 @@ ucode_gen_files_intel(uint8_t *buf, int size, char *path)
 			dprintf("proc_flags = %x, platid = %x, name = %s\n",
 			    uhp->uh_proc_flags, platid, name);
 
-			if (ucode_should_update_intel(name, uhp->uh_rev) != 0) {
-
+			if (ucode_should_update_intel(name,
+			    uhp->uh_rev) != 0) {
 				/* Remove the existing one first */
 				(void) unlink(name);
-
 				if (link(firstname, name) == -1) {
 					ucode_perror(name, EM_SYS);
 					return (EM_SYS);
@@ -422,12 +421,15 @@ ucode_gen_files_intel(uint8_t *buf, int size, char *path)
 					continue;
 
 				(void) snprintf(name, PATH_MAX,
-				    "%s/%08X-%02X", path, extp->uet_ext_sig[i],
-				    id);
+				    "%s/%08X-%02X", path,
+				    uesp->ues_signature, id);
 
-				if (ucode_should_update_intel(name, uhp->uh_rev)
-				    != 0) {
+				dprintf("extsig: proc_flags = %x, "
+				    "platid = %x, name = %s\n",
+				    uesp->ues_proc_flags, id, name);
 
+				if (ucode_should_update_intel(name,
+				    uhp->uh_rev) != 0) {
 					/* Remove the existing one first */
 					(void) unlink(name);
 					if (link(firstname, name) == -1) {
diff --git a/usr/src/common/ucode/ucode_errno.h b/usr/src/common/ucode/ucode_errno.h
index 96471142df..ba371ff523 100644
--- a/usr/src/common/ucode/ucode_errno.h
+++ b/usr/src/common/ucode/ucode_errno.h
@@ -22,13 +22,13 @@
 /*
  * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
+ *
+ * Copyright 2021 OmniOS Community Edition (OmniOSce) Association.
  */
 
 #ifndef _UCODE_ERRNO_H
 #define	_UCODE_ERRNO_H
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #ifdef	__cplusplus
 extern "C" {
 #endif
@@ -40,6 +40,8 @@ typedef enum ucode_errno {
 	EM_FILEFORMAT,	/* Not a valid microcode file */
 	EM_HEADER,	/* File header is invalid */
 	EM_CHECKSUM,	/* Checksum is invalid */
+	EM_EXTCHECKSUM,	/* Extended signature table checksum is invalid */
+	EM_SIGCHECKSUM,	/* Extended signature checksum is invalid */
 	EM_INVALIDARG,	/* Invalid argument(s) */
 	EM_NOMATCH,	/* No matching microcode found */
 	EM_HIGHERREV,	/* File does not contain higher revision microcode */
diff --git a/usr/src/common/ucode/ucode_utils.c b/usr/src/common/ucode/ucode_utils.c
index ec3efa63ee..79c2669413 100644
--- a/usr/src/common/ucode/ucode_utils.c
+++ b/usr/src/common/ucode/ucode_utils.c
@@ -37,7 +37,7 @@
 /*
  * Refer to
  *	Intel 64 and IA-32 Architectures Software Developers's Manual
- *		Chapter 9.11 Microcode Update Facilities
+ *		Chapter 9.11 Microcode Update Facilities	[1]
  * for details.
  */
 
@@ -116,6 +116,31 @@ ucode_checksum_intel(uint32_t sum, uint32_t size, uint8_t *code)
 	return (sum);
 }
 
+uint32_t
+ucode_checksum_intel_extsig(ucode_header_intel_t *uhp,
+    ucode_ext_sig_intel_t *uesp)
+{
+	/*
+	 * From [1], table 9-7, the checksum field contained in the extended
+	 * patch signature is the checksum across the entire update which would
+	 * result if the primary processor signature and processor flags were
+	 * replaced with the values from this entry.
+	 *
+	 * We can therefore just calculate the difference in the checksum
+	 * between the old and new values and return that to the caller. If the
+	 * difference is zero then the checksum for the patch is valid.
+	 */
+	uint32_t diff;
+
+	diff = uesp->ues_signature +
+	    uesp->ues_proc_flags + uesp->ues_checksum;
+	diff -= uhp->uh_signature + uhp->uh_proc_flags +
+	    uhp->uh_checksum;
+
+	return (diff);
+}
+
+
 ucode_errno_t
 ucode_validate_amd(uint8_t *ucodep, int size)
 {
@@ -177,37 +202,35 @@ ucode_validate_intel(uint8_t *ucodep, int size)
 		uint8_t *curbuf = &ucodep[size - remaining];
 		ucode_errno_t rc;
 
-		uhp = (ucode_header_intel_t *)(intptr_t)curbuf;
+		uhp = (ucode_header_intel_t *)curbuf;
 
 		if ((rc = ucode_header_validate_intel(uhp)) != EM_OK)
 			return (rc);
 
 		total_size = UCODE_TOTAL_SIZE_INTEL(uhp->uh_total_size);
 
-		if (ucode_checksum_intel(0, total_size, curbuf))
+		if (ucode_checksum_intel(0, total_size, curbuf) != 0)
 			return (EM_CHECKSUM);
 
 		body_size = UCODE_BODY_SIZE_INTEL(uhp->uh_body_size);
 		ext_size = total_size - (header_size + body_size);
 
 		if (ext_size > 0) {
+			ucode_ext_table_intel_t *ext;
 			uint32_t i;
 
-			if (ucode_checksum_intel(0, ext_size,
-			    &curbuf[header_size + body_size])) {
-				return (EM_CHECKSUM);
-			}
+			ext = (ucode_ext_table_intel_t *)
+			    &curbuf[header_size + body_size];
+
+			if (ucode_checksum_intel(0, ext_size, (uint8_t *)ext))
+				return (EM_EXTCHECKSUM);
 
-			ext_size -= UCODE_EXT_TABLE_SIZE_INTEL;
-			for (i = 0; i < ext_size / UCODE_EXT_SIG_SIZE_INTEL;
-			    i++) {
-				if (ucode_checksum_intel(0,
-				    UCODE_EXT_SIG_SIZE_INTEL,
-				    &curbuf[total_size - ext_size +
-				    i * UCODE_EXT_SIG_SIZE_INTEL])) {
+			for (i = 0; i < ext->uet_count; i++) {
+				ucode_ext_sig_intel_t *sig =
+				    &ext->uet_ext_sig[i];
 
-					return (EM_CHECKSUM);
-				}
+				if (ucode_checksum_intel_extsig(uhp, sig) != 0)
+					return (EM_SIGCHECKSUM);
 			}
 		}
 
diff --git a/usr/src/uts/common/sys/ucode.h b/usr/src/uts/common/sys/ucode.h
index 0e1b60cc53..50db604b48 100644
--- a/usr/src/uts/common/sys/ucode.h
+++ b/usr/src/uts/common/sys/ucode.h
@@ -226,6 +226,8 @@ typedef union ucode_file {
 
 extern ucode_errno_t ucode_header_validate_intel(ucode_header_intel_t *);
 extern uint32_t ucode_checksum_intel(uint32_t, uint32_t, uint8_t *);
+extern uint32_t ucode_checksum_intel_extsig(ucode_header_intel_t *,
+    ucode_ext_sig_intel_t *);
 
 extern ucode_errno_t ucode_validate_amd(uint8_t *, int);
 extern ucode_errno_t ucode_validate_intel(uint8_t *, int);
diff --git a/usr/src/uts/i86pc/os/microcode.c b/usr/src/uts/i86pc/os/microcode.c
index d07a79bf18..cbcecadbef 100644
--- a/usr/src/uts/i86pc/os/microcode.c
+++ b/usr/src/uts/i86pc/os/microcode.c
@@ -25,6 +25,7 @@
  *
  * Copyright 2012 Nexenta Systems, Inc. All rights reserved.
  * Copyright (c) 2018, Joyent, Inc.
+ * Copyright 2021 OmniOS Community Edition (OmniOSce) Association.
  */
 
 #include <sys/asm_linkage.h>
@@ -609,18 +610,18 @@ ucode_locate_intel(cpu_t *cp, cpu_ucode_info_t *uinfop, ucode_file_t *ufp)
 			rc = EM_FILESIZE;
 		} else if (ucode_checksum_intel(0, ext_size,
 		    (uint8_t *)(ucodefp->uf_ext_table))) {
-			rc = EM_CHECKSUM;
+			rc = EM_EXTCHECKSUM;
 		} else {
 			int i;
 
-			ext_size -= UCODE_EXT_TABLE_SIZE_INTEL;
-			for (i = 0; i < ucodefp->uf_ext_table->uet_count;
-			    i++) {
-				if (ucode_checksum_intel(0,
-				    UCODE_EXT_SIG_SIZE_INTEL,
-				    (uint8_t *)(&(ucodefp->uf_ext_table->
-				    uet_ext_sig[i])))) {
-					rc = EM_CHECKSUM;
+			for (i = 0; i < ucodefp->uf_ext_table->uet_count; i++) {
+				ucode_ext_sig_intel_t *sig;
+
+				sig = &ucodefp->uf_ext_table->uet_ext_sig[i];
+
+				if (ucode_checksum_intel_extsig(uhp,
+				    sig) != 0) {
+					rc = EM_SIGCHECKSUM;
 					break;
 				}
 			}