diff mbox series

[v7,06/12] platform/x86/intel/ifs: Check IFS Image sanity

Message ID 20220506225410.1652287-7-tony.luck@intel.com (mailing list archive)
State Accepted, archived
Headers show
Series Introduce In Field Scan driver | expand

Commit Message

Luck, Tony May 6, 2022, 10:54 p.m. UTC
From: Jithu Joseph <jithu.joseph@intel.com>

IFS image is designed specifically for a given family, model and
stepping of the processor. Like Intel microcode header, the IFS image
has the Processor Signature, Checksum and Processor Flags that must be
matched with the information returned by the CPUID.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Co-developed-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
Acked-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/platform/x86/intel/ifs/load.c | 66 +++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

Comments

Thomas Gleixner May 9, 2022, 12:11 p.m. UTC | #1
On Fri, May 06 2022 at 15:54, Tony Luck wrote:
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> Co-developed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> Acked-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Borislav Petkov May 9, 2022, 4:31 p.m. UTC | #2
On Fri, May 06, 2022 at 03:54:04PM -0700, Tony Luck wrote:
> From: Jithu Joseph <jithu.joseph@intel.com>
> 
> IFS image is designed specifically for a given family, model and
> stepping of the processor. Like Intel microcode header, the IFS image
> has the Processor Signature, Checksum and Processor Flags that must be
> matched with the information returned by the CPUID.

Is the checksum the only protection against people loading arbitrary IFS
images or are those things signed or encrypted, just like the microcode?

I'd hope they pass the same checks as microcode, when they get loaded,
considering the similarity of how they're handled...
Luck, Tony May 9, 2022, 4:51 p.m. UTC | #3
> Is the checksum the only protection against people loading arbitrary IFS
> images or are those things signed or encrypted, just like the microcode?
>
> I'd hope they pass the same checks as microcode, when they get loaded,
> considering the similarity of how they're handled...

The checksum is just a "did this file get corrupted check". The file contains
SHA256 checksums for each of the chunks. These checksums are digitally
signed. Checking of these is done by microcode when the file is loaded into
BIOS reserved memory (where it is inaccessible to OS and I/O).

-Tony
Borislav Petkov May 9, 2022, 4:56 p.m. UTC | #4
On Mon, May 09, 2022 at 04:51:56PM +0000, Luck, Tony wrote:
> The checksum is just a "did this file get corrupted check". The file contains
> SHA256 checksums for each of the chunks. These checksums are digitally
> signed. Checking of these is done by microcode when the file is loaded into
> BIOS reserved memory (where it is inaccessible to OS and I/O).

That sounds like something I was hoping to hear, good. :-)

Thx.
diff mbox series

Patch

diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 9fb71d38c819..cfbf62494c89 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -2,9 +2,72 @@ 
 /* Copyright(c) 2022 Intel Corporation. */
 
 #include <linux/firmware.h>
+#include <asm/cpu.h>
+#include <asm/microcode_intel.h>
 
 #include "ifs.h"
 
+static int ifs_sanity_check(struct device *dev,
+			    const struct microcode_header_intel *mc_header)
+{
+	unsigned long total_size, data_size;
+	u32 sum, *mc;
+
+	total_size = get_totalsize(mc_header);
+	data_size = get_datasize(mc_header);
+
+	if ((data_size + MC_HEADER_SIZE > total_size) || (total_size % sizeof(u32))) {
+		dev_err(dev, "bad ifs data file size.\n");
+		return -EINVAL;
+	}
+
+	if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
+		dev_err(dev, "invalid/unknown ifs update format.\n");
+		return -EINVAL;
+	}
+
+	mc = (u32 *)mc_header;
+	sum = 0;
+	for (int i = 0; i < total_size / sizeof(u32); i++)
+		sum += mc[i];
+
+	if (sum) {
+		dev_err(dev, "bad ifs data checksum, aborting.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static bool find_ifs_matching_signature(struct device *dev, struct ucode_cpu_info *uci,
+					const struct microcode_header_intel *shdr)
+{
+	unsigned int mc_size;
+
+	mc_size = get_totalsize(shdr);
+
+	if (!mc_size || ifs_sanity_check(dev, shdr) < 0) {
+		dev_err(dev, "ifs sanity check failure\n");
+		return false;
+	}
+
+	if (!intel_cpu_signatures_match(uci->cpu_sig.sig, uci->cpu_sig.pf, shdr->sig, shdr->pf)) {
+		dev_err(dev, "ifs signature, pf not matching\n");
+		return false;
+	}
+
+	return true;
+}
+
+static bool ifs_image_sanity_check(struct device *dev, const struct microcode_header_intel *data)
+{
+	struct ucode_cpu_info uci;
+
+	intel_cpu_collect_info(&uci);
+
+	return find_ifs_matching_signature(dev, &uci, data);
+}
+
 /*
  * Load ifs image. Before loading ifs module, the ifs image must be located
  * in /lib/firmware/intel/ifs and named as {family/model/stepping}.{testname}.
@@ -24,5 +87,8 @@  void ifs_load_firmware(struct device *dev)
 		return;
 	}
 
+	if (!ifs_image_sanity_check(dev, (struct microcode_header_intel *)fw->data))
+		dev_err(dev, "ifs header sanity check failed\n");
+
 	release_firmware(fw);
 }