| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20220829180436.716672-1-ashok.raj@intel.com>
Date: Mon, 29 Aug 2022 18:04:36 +0000
From: Ashok Raj <ashok.raj@...el.com>
To: Borislav Petkov <bp@...en8.de>,
Thomas Gleixner <tglx@...utronix.de>
Cc: "LKML Mailing List" <linux-kernel@...r.kernel.org>,
X86-kernel <x86@...nel.org>, Dave Hansen <dave.hansen@...el.com>,
Andy Lutomirski <luto@...capital.net>,
Ingo Molnar <mingo@...nel.org>,
"Ashok Raj" <ashok.raj@...el.com>,
Tom Lendacky <thomas.lendacky@....com>,
Tony Luck <tony.luck@...el.com>
Subject: [PATCH] x86/microcode/intel: Allow late loading only if a min rev is specified
In general users don't have the necessary information to determine
whether a late loading of a new microcode version has removed any feature
(MSR, CPUID etc) between what is currently loaded and this new microcode.
To address this issue, Intel has added a "minimum required version" field to
a previously reserved field in the file header. Microcode updates
should only be applied if the current microcode version is equal
to, or greater than this minimum required version.
Thomas made some suggestions[1] on how meta-data in the microcode file could
provide Linux with information to decide if the new microcode is suitable
candidate for late loading. But even the "simpler" option#1 requires a lot of
metadata and corresponding kernel code to parse it.
The proposal here is an even simpler option. Simply "OS visible features"
such as CPUID and MSRs are the only two examples. The microcode must not
change these OS visible features because they cause problems after late
loading.
Pseudo code for late loading is as follows:
if header.min_required_id == 0
This is old format microcode, block late loading
else if current_ucode_version < header.min_required_id
Current version is too old, block late loading of this microcode.
else
OK to proceed with late loading.
Any microcode that modifies the interface to an OS-visible feature
will set the min_version to itself. This will enforce this microcode is
not suitable for late loading unless the currently loaded revision is greater
or equal to the new microcode affecting the change.
The enforcement is not in hardware and limited to kernel loader enforcing
the requirement. It is not required for early loading of microcode to
enforce this requirement, since the new features are only
evaluated after early loading in the boot process.
Test cases covered:
1. With new kernel, attempting to load an older format microcode with the
min_rev=0 should be blocked by kernel.
[ 210.541802] Late loading denied: microcode header does not specify a
required min version.
2. New microcode with a non-zero min_rev in the header, but the specified
min_rev is greater than what is currently loaded in the CPU should be
blocked by kernel.
245.139828] microcode: Late loading denied: Current revision 0x8f685300 is too old to update, must be at 0xaa000050 version or higher. Use early loading instead.
3. New microcode with a min_rev < currently loaded should allow loading the
microcode
4. Build initrd with microcode that has min_rev=0, or min_rev > currently
loaded should permit early loading microcode from initrd.
[1] https://lore.kernel.org/linux-kernel/alpine.DEB.2.21.1909062237580.1902@nanos.tec.linutronix.de/
Tested-by: William Xie <william.xie@...el.com>
Reviewed-by: Tony Luck <tony.luck@...el.com>
Signed-off-by: Ashok Raj <ashok.raj@...el.com>
---
Needs:
https://lore.kernel.org/lkml/20220825075445.28171-1-bp@alien8.de/
v3: https://lore.kernel.org/lkml/20220817051127.3323755-1-ashok.raj@intel.com/
v2: https://lore.kernel.org/lkml/20220816043754.3258815-1-ashok.raj@intel.com/
v1: https://lore.kernel.org/lkml/20220813223825.3164861-1-ashok.raj@intel.com/
v3->v4:
- Drop the multi-step patch, provide that as a separate series later, since
we want make more changes to mulit-step handling.
- Boris:
* Rename fw_refresh as late_loading, don't use print_err as indicator
for late loading
* Changed commit and warn messages as suggested.
---
arch/x86/include/asm/microcode.h | 2 +-
arch/x86/include/asm/microcode_intel.h | 4 +++-
arch/x86/kernel/cpu/microcode/amd.c | 4 ++--
arch/x86/kernel/cpu/microcode/intel.c | 31 +++++++++++++++++++++-----
4 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index 7f7800e15ed0..9df733b35912 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -33,7 +33,7 @@ enum ucode_state {
struct microcode_ops {
enum ucode_state (*request_microcode_fw) (int cpu, struct device *,
- bool refresh_fw);
+ bool late_loading);
void (*microcode_fini_cpu) (int cpu);
diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 4c92cea7e4b5..16b8715e0984 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -14,7 +14,9 @@ struct microcode_header_intel {
unsigned int pf;
unsigned int datasize;
unsigned int totalsize;
- unsigned int reserved[3];
+ unsigned int reserved1;
+ unsigned int min_req_id;
+ unsigned int reserved3;
};
struct microcode_intel {
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 5f38dd75cbc5..c18d3f01a452 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -891,7 +891,7 @@ load_microcode_amd(bool save, u8 family, const u8 *data, size_t size)
* These might be larger than 2K.
*/
static enum ucode_state request_microcode_amd(int cpu, struct device *device,
- bool refresh_fw)
+ bool late_loading)
{
char fw_name[36] = "amd-ucode/microcode_amd.bin";
struct cpuinfo_x86 *c = &cpu_data(cpu);
@@ -900,7 +900,7 @@ static enum ucode_state request_microcode_amd(int cpu, struct device *device,
const struct firmware *fw;
/* reload ucode container only on the boot cpu */
- if (!refresh_fw || !bsp)
+ if (!late_loading || !bsp)
return UCODE_OK;
if (c->x86 >= 0x15)
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 1fcbd671f1df..332ba19e0147 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -163,13 +163,14 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
intel_ucode_patch = p->data;
}
-static int microcode_sanity_check(void *mc, int print_err)
+static int microcode_sanity_check(void *mc, int print_err, bool late_loading)
{
unsigned long total_size, data_size, ext_table_size;
struct microcode_header_intel *mc_header = mc;
struct extended_sigtable *ext_header = NULL;
u32 sum, orig_sum, ext_sigcount = 0, i;
struct extended_signature *ext_sig;
+ struct ucode_cpu_info uci;
total_size = get_totalsize(mc_header);
data_size = get_datasize(mc_header);
@@ -240,6 +241,24 @@ static int microcode_sanity_check(void *mc, int print_err)
return -EINVAL;
}
+
+ /*
+ * Enforce for late-load that min_req_id is specified in the
+ * header. Otherwise its an old format microcode, reject it
+ */
+ if (late_loading) {
+ if (!mc_header->min_req_id) {
+ pr_warn("Late loading denied: Microcode header does not specify a required min version\n");
+ return -EINVAL;
+ }
+ intel_cpu_collect_info(&uci);
+ if (uci.cpu_sig.rev < mc_header->min_req_id) {
+ pr_warn("Late loading denied: Current revision 0x%x too old to update, must be at 0x%x or higher. Use early loading instead\n",
+ uci.cpu_sig.rev, mc_header->min_req_id);
+ return -EINVAL;
+ }
+ }
+
if (!ext_table_size)
return 0;
@@ -281,7 +300,7 @@ scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
mc_size = get_totalsize(mc_header);
if (!mc_size ||
mc_size > size ||
- microcode_sanity_check(data, 0) < 0)
+ microcode_sanity_check(data, 0, false) < 0)
break;
size -= mc_size;
@@ -778,7 +797,7 @@ static enum ucode_state apply_microcode_intel(int cpu)
return ret;
}
-static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
+static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter, bool late_loading)
{
struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
unsigned int curr_mc_size = 0, new_mc_size = 0;
@@ -820,7 +839,7 @@ static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
memcpy(mc, &mc_header, sizeof(mc_header));
data = mc + sizeof(mc_header);
if (!copy_from_iter_full(data, data_size, iter) ||
- microcode_sanity_check(mc, 1) < 0) {
+ microcode_sanity_check(mc, 1, late_loading) < 0) {
break;
}
@@ -886,7 +905,7 @@ static bool is_blacklisted(unsigned int cpu)
}
static enum ucode_state request_microcode_fw(int cpu, struct device *device,
- bool refresh_fw)
+ bool late_loading)
{
struct cpuinfo_x86 *c = &cpu_data(cpu);
const struct firmware *firmware;
@@ -909,7 +928,7 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device,
kvec.iov_base = (void *)firmware->data;
kvec.iov_len = firmware->size;
iov_iter_kvec(&iter, WRITE, &kvec, 1, firmware->size);
- ret = generic_load_microcode(cpu, &iter);
+ ret = generic_load_microcode(cpu, &iter, late_loading);
release_firmware(firmware);
--
2.32.0
Powered by blists - more mailing lists