* [PATCH 0/3] x86/microcode: Some fixes @ 2019-04-05 13:30 Borislav Petkov 2019-04-05 13:30 ` [PATCH 1/3] x86/microcode/intel: Refactor Intel microcode blob loading Borislav Petkov ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Borislav Petkov @ 2019-04-05 13:30 UTC (permalink / raw) To: LKML; +Cc: Jann Horn, X86 ML From: Borislav Petkov <bp@suse.de> Hi all, here is a nice cleanup from Jann along with a long overdue deprecation of /dev/cpu/microcode. So overdue that it was broken for more than a year and it went unnoticed. A very good sign that it can safely go to the eternal fields of dead electrons. Thx. Borislav Petkov (2): x86/microcode: Fix the ancient deprecated microcode loading method x86/microcode: Deprecate MICROCODE_OLD_INTERFACE Jann Horn (1): x86/microcode/intel: Refactor Intel microcode blob loading arch/x86/Kconfig | 10 +++- arch/x86/kernel/cpu/microcode/core.c | 3 +- arch/x86/kernel/cpu/microcode/intel.c | 71 ++++++++++++++------------- 3 files changed, 47 insertions(+), 37 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] x86/microcode/intel: Refactor Intel microcode blob loading 2019-04-05 13:30 [PATCH 0/3] x86/microcode: Some fixes Borislav Petkov @ 2019-04-05 13:30 ` Borislav Petkov 2019-04-05 21:59 ` Thomas Gleixner 2019-04-05 13:30 ` [PATCH 2/3] x86/microcode: Fix the ancient deprecated microcode loading method Borislav Petkov 2019-04-05 13:30 ` [PATCH 3/3] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE Borislav Petkov 2 siblings, 1 reply; 9+ messages in thread From: Borislav Petkov @ 2019-04-05 13:30 UTC (permalink / raw) To: LKML; +Cc: Jann Horn, X86 ML, H. Peter Anvin, Ingo Molnar, Thomas Gleixner From: Jann Horn <jannh@google.com> Change generic_load_microcode() to use the iov_iter API instead of a clumsy open-coded version which has to pay attention to __user data or kernel data, depending on the loading method. This allows to avoid explicit casting between user and kernel pointers. Because the iov_iter API makes it hard to read the same location twice, as a side effect, also fix a double-read of the microcode header (which could e.g. lead to out-of-bounds reads in microcode_sanity_check()). Not that it matters much, only root is allowed to load microcode anyway... [ bp: Massage a bit, sort function-local variables. ] Signed-off-by: Jann Horn <jannh@google.com> Signed-off-by: Borislav Petkov <bp@suse.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: x86-ml <x86@kernel.org> Link: https://lkml.kernel.org/r/20190404111128.131157-1-jannh@google.com --- arch/x86/kernel/cpu/microcode/intel.c | 71 ++++++++++++++------------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 16936a24795c..a44bdbe7c55e 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -31,6 +31,7 @@ #include <linux/kernel.h> #include <linux/slab.h> #include <linux/cpu.h> +#include <linux/uio.h> #include <linux/mm.h> #include <asm/microcode_intel.h> @@ -861,32 +862,33 @@ static enum ucode_state apply_microcode_intel(int cpu) return ret; } -static enum ucode_state generic_load_microcode(int cpu, void *data, size_t size, - int (*get_ucode_data)(void *, const void *, size_t)) +static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter) { struct ucode_cpu_info *uci = ucode_cpu_info + cpu; - u8 *ucode_ptr = data, *new_mc = NULL, *mc = NULL; - int new_rev = uci->cpu_sig.rev; - unsigned int leftover = size; unsigned int curr_mc_size = 0, new_mc_size = 0; - unsigned int csig, cpf; enum ucode_state ret = UCODE_OK; + int new_rev = uci->cpu_sig.rev; + u8 *new_mc = NULL, *mc = NULL; + unsigned int csig, cpf; - while (leftover) { + while (iov_iter_count(iter)) { struct microcode_header_intel mc_header; - unsigned int mc_size; + unsigned int mc_size, data_size; + u8 *data; - if (leftover < sizeof(mc_header)) { - pr_err("error! Truncated header in microcode data file\n"); + if (!copy_from_iter_full(&mc_header, sizeof(mc_header), iter)) { + pr_err("error! Truncated or inaccessible header in microcode data file\n"); break; } - if (get_ucode_data(&mc_header, ucode_ptr, sizeof(mc_header))) - break; - mc_size = get_totalsize(&mc_header); - if (!mc_size || mc_size > leftover) { - pr_err("error! Bad data in microcode data file\n"); + if (mc_size < sizeof(mc_header)) { + pr_err("error! Bad data in microcode data file (totalsize too small)\n"); + break; + } + data_size = mc_size - sizeof(mc_header); + if (data_size > iov_iter_count(iter)) { + pr_err("error! Bad data in microcode data file (truncated file?)\n"); break; } @@ -899,7 +901,9 @@ static enum ucode_state generic_load_microcode(int cpu, void *data, size_t size, curr_mc_size = mc_size; } - if (get_ucode_data(mc, ucode_ptr, mc_size) || + 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) { break; } @@ -914,14 +918,11 @@ static enum ucode_state generic_load_microcode(int cpu, void *data, size_t size, mc = NULL; /* trigger new vmalloc */ ret = UCODE_NEW; } - - ucode_ptr += mc_size; - leftover -= mc_size; } vfree(mc); - if (leftover) { + if (iov_iter_count(iter)) { vfree(new_mc); return UCODE_ERROR; } @@ -945,12 +946,6 @@ static enum ucode_state generic_load_microcode(int cpu, void *data, size_t size, return ret; } -static int get_ucode_fw(void *to, const void *from, size_t n) -{ - memcpy(to, from, n); - return 0; -} - static bool is_blacklisted(unsigned int cpu) { struct cpuinfo_x86 *c = &cpu_data(cpu); @@ -977,10 +972,12 @@ static bool is_blacklisted(unsigned int cpu) static enum ucode_state request_microcode_fw(int cpu, struct device *device, bool refresh_fw) { - char name[30]; struct cpuinfo_x86 *c = &cpu_data(cpu); const struct firmware *firmware; + struct iov_iter iter; enum ucode_state ret; + struct kvec kvec; + char name[30]; if (is_blacklisted(cpu)) return UCODE_NFOUND; @@ -993,26 +990,30 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device, return UCODE_NFOUND; } - ret = generic_load_microcode(cpu, (void *)firmware->data, - firmware->size, &get_ucode_fw); + 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); release_firmware(firmware); return ret; } -static int get_ucode_user(void *to, const void *from, size_t n) -{ - return copy_from_user(to, from, n); -} - static enum ucode_state request_microcode_user(int cpu, const void __user *buf, size_t size) { + struct iov_iter iter; + struct iovec iov; + if (is_blacklisted(cpu)) return UCODE_NFOUND; - return generic_load_microcode(cpu, (void *)buf, size, &get_ucode_user); + iov.iov_base = (void __user *)buf; + iov.iov_len = size; + iov_iter_init(&iter, WRITE, &iov, 1, size); + + return generic_load_microcode(cpu, &iter); } static struct microcode_ops microcode_intel_ops = { -- 2.21.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] x86/microcode/intel: Refactor Intel microcode blob loading 2019-04-05 13:30 ` [PATCH 1/3] x86/microcode/intel: Refactor Intel microcode blob loading Borislav Petkov @ 2019-04-05 21:59 ` Thomas Gleixner 0 siblings, 0 replies; 9+ messages in thread From: Thomas Gleixner @ 2019-04-05 21:59 UTC (permalink / raw) To: Borislav Petkov; +Cc: LKML, Jann Horn, X86 ML, H. Peter Anvin, Ingo Molnar On Fri, 5 Apr 2019, Borislav Petkov wrote: > From: Jann Horn <jannh@google.com> > > Change generic_load_microcode() to use the iov_iter API instead of a > clumsy open-coded version which has to pay attention to __user data > or kernel data, depending on the loading method. This allows to avoid > explicit casting between user and kernel pointers. > > Because the iov_iter API makes it hard to read the same location twice, > as a side effect, also fix a double-read of the microcode header (which > could e.g. lead to out-of-bounds reads in microcode_sanity_check()). > > Not that it matters much, only root is allowed to load microcode > anyway... Nice! Reviewed-by: Thomas Gleixner <tglx@linutronix.de> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] x86/microcode: Fix the ancient deprecated microcode loading method 2019-04-05 13:30 [PATCH 0/3] x86/microcode: Some fixes Borislav Petkov 2019-04-05 13:30 ` [PATCH 1/3] x86/microcode/intel: Refactor Intel microcode blob loading Borislav Petkov @ 2019-04-05 13:30 ` Borislav Petkov 2019-04-05 21:55 ` Thomas Gleixner 2019-04-10 20:49 ` [tip:x86/microcode] " tip-bot for Borislav Petkov 2019-04-05 13:30 ` [PATCH 3/3] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE Borislav Petkov 2 siblings, 2 replies; 9+ messages in thread From: Borislav Petkov @ 2019-04-05 13:30 UTC (permalink / raw) To: LKML; +Cc: Jann Horn, X86 ML From: Borislav Petkov <bp@suse.de> The commit in Fixes: added the new define UCODE_NEW to denote that an update should happen only when newer microcode (than installed on the system) has been found. But it missed adjusting that for the old /dev/cpu/microcode loading interface. Fix it. Fixes: 2613f36ed965 ("x86/microcode: Attempt late loading only when new microcode is present") Signed-off-by: Borislav Petkov <bp@suse.de> Cc: Jann Horn <jannh@google.com> --- arch/x86/kernel/cpu/microcode/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index 5260185cbf7b..8a4a7823451a 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -418,8 +418,9 @@ static int do_microcode_update(const void __user *buf, size_t size) if (ustate == UCODE_ERROR) { error = -1; break; - } else if (ustate == UCODE_OK) + } else if (ustate == UCODE_NEW) { apply_microcode_on_target(cpu); + } } return error; -- 2.21.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] x86/microcode: Fix the ancient deprecated microcode loading method 2019-04-05 13:30 ` [PATCH 2/3] x86/microcode: Fix the ancient deprecated microcode loading method Borislav Petkov @ 2019-04-05 21:55 ` Thomas Gleixner 2019-04-10 20:49 ` [tip:x86/microcode] " tip-bot for Borislav Petkov 1 sibling, 0 replies; 9+ messages in thread From: Thomas Gleixner @ 2019-04-05 21:55 UTC (permalink / raw) To: Borislav Petkov; +Cc: LKML, Jann Horn, X86 ML On Fri, 5 Apr 2019, Borislav Petkov wrote: > From: Borislav Petkov <bp@suse.de> > > The commit in Fixes: added the new define UCODE_NEW to denote that an That reads odd. What's wrong with: Commit 2613f36ed965 added .... or A recent commit added mm? > update should happen only when newer microcode (than installed on the > system) has been found. > > But it missed adjusting that for the old /dev/cpu/microcode loading > interface. Fix it. > > Fixes: 2613f36ed965 ("x86/microcode: Attempt late loading only when new microcode is present") > Signed-off-by: Borislav Petkov <bp@suse.de> > Cc: Jann Horn <jannh@google.com> Other than that: Reviewed-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/kernel/cpu/microcode/core.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c > index 5260185cbf7b..8a4a7823451a 100644 > --- a/arch/x86/kernel/cpu/microcode/core.c > +++ b/arch/x86/kernel/cpu/microcode/core.c > @@ -418,8 +418,9 @@ static int do_microcode_update(const void __user *buf, size_t size) > if (ustate == UCODE_ERROR) { > error = -1; > break; > - } else if (ustate == UCODE_OK) > + } else if (ustate == UCODE_NEW) { > apply_microcode_on_target(cpu); > + } > } > > return error; > -- > 2.21.0 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [tip:x86/microcode] x86/microcode: Fix the ancient deprecated microcode loading method 2019-04-05 13:30 ` [PATCH 2/3] x86/microcode: Fix the ancient deprecated microcode loading method Borislav Petkov 2019-04-05 21:55 ` Thomas Gleixner @ 2019-04-10 20:49 ` tip-bot for Borislav Petkov 1 sibling, 0 replies; 9+ messages in thread From: tip-bot for Borislav Petkov @ 2019-04-10 20:49 UTC (permalink / raw) To: linux-tip-commits; +Cc: tglx, jannh, linux-kernel, bp, hpa, mingo Commit-ID: 24613a04ad1c0588c10f4b5403ca60a73d164051 Gitweb: https://git.kernel.org/tip/24613a04ad1c0588c10f4b5403ca60a73d164051 Author: Borislav Petkov <bp@suse.de> AuthorDate: Thu, 4 Apr 2019 22:14:07 +0200 Committer: Borislav Petkov <bp@suse.de> CommitDate: Wed, 10 Apr 2019 22:41:24 +0200 x86/microcode: Fix the ancient deprecated microcode loading method Commit 2613f36ed965 ("x86/microcode: Attempt late loading only when new microcode is present") added the new define UCODE_NEW to denote that an update should happen only when newer microcode (than installed on the system) has been found. But it missed adjusting that for the old /dev/cpu/microcode loading interface. Fix it. Fixes: 2613f36ed965 ("x86/microcode: Attempt late loading only when new microcode is present") Signed-off-by: Borislav Petkov <bp@suse.de> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Cc: Jann Horn <jannh@google.com> Link: https://lkml.kernel.org/r/20190405133010.24249-3-bp@alien8.de --- arch/x86/kernel/cpu/microcode/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index 5260185cbf7b..8a4a7823451a 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -418,8 +418,9 @@ static int do_microcode_update(const void __user *buf, size_t size) if (ustate == UCODE_ERROR) { error = -1; break; - } else if (ustate == UCODE_OK) + } else if (ustate == UCODE_NEW) { apply_microcode_on_target(cpu); + } } return error; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE 2019-04-05 13:30 [PATCH 0/3] x86/microcode: Some fixes Borislav Petkov 2019-04-05 13:30 ` [PATCH 1/3] x86/microcode/intel: Refactor Intel microcode blob loading Borislav Petkov 2019-04-05 13:30 ` [PATCH 2/3] x86/microcode: Fix the ancient deprecated microcode loading method Borislav Petkov @ 2019-04-05 13:30 ` Borislav Petkov 2019-04-05 21:53 ` Thomas Gleixner 2019-04-10 20:50 ` [tip:x86/microcode] " tip-bot for Borislav Petkov 2 siblings, 2 replies; 9+ messages in thread From: Borislav Petkov @ 2019-04-05 13:30 UTC (permalink / raw) To: LKML; +Cc: Jann Horn, X86 ML From: Borislav Petkov <bp@suse.de> This is the ancient loading interface which requires special tools to be used. The bigger problem with it is that it is as inadequate for proper loading of microcode as the late microcode loading interface is because it happens just as late. iucode_tool's manpage already warns people that it is deprecated. Deprecate it and turn it off by default along with a big warning in the Kconfig help. It will go away sometime in the future. Signed-off-by: Borislav Petkov <bp@suse.de> --- arch/x86/Kconfig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 5ad92419be19..5a0a752f3ddd 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1330,8 +1330,16 @@ config MICROCODE_AMD processors will be enabled. config MICROCODE_OLD_INTERFACE - def_bool y + bool "Ancient loading interface (DEPRECATED)" + default n depends on MICROCODE + ---help--- + DO NOT USE THIS! This is the ancient /dev/cpu/microcode interface + which was used by userspace tools like iucode_tool and microcode.ctl. + It is inadequate because it runs too late to be able to properly + load microcode on a machine and it needs special tools. Instead, you + should've switched to the early loading method with the initrd or + builtin microcode by now: Documentation/x86/microcode.txt config X86_MSR tristate "/dev/cpu/*/msr - Model-specific register support" -- 2.21.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE 2019-04-05 13:30 ` [PATCH 3/3] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE Borislav Petkov @ 2019-04-05 21:53 ` Thomas Gleixner 2019-04-10 20:50 ` [tip:x86/microcode] " tip-bot for Borislav Petkov 1 sibling, 0 replies; 9+ messages in thread From: Thomas Gleixner @ 2019-04-05 21:53 UTC (permalink / raw) To: Borislav Petkov; +Cc: LKML, Jann Horn, X86 ML On Fri, 5 Apr 2019, Borislav Petkov wrote: > From: Borislav Petkov <bp@suse.de> > > This is the ancient loading interface which requires special tools to be > used. The bigger problem with it is that it is as inadequate for proper > loading of microcode as the late microcode loading interface is because > it happens just as late. > > iucode_tool's manpage already warns people that it is deprecated. > > Deprecate it and turn it off by default along with a big warning in the > Kconfig help. It will go away sometime in the future. > > Signed-off-by: Borislav Petkov <bp@suse.de> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [tip:x86/microcode] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE 2019-04-05 13:30 ` [PATCH 3/3] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE Borislav Petkov 2019-04-05 21:53 ` Thomas Gleixner @ 2019-04-10 20:50 ` tip-bot for Borislav Petkov 1 sibling, 0 replies; 9+ messages in thread From: tip-bot for Borislav Petkov @ 2019-04-10 20:50 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, bp, hpa, mingo, tglx Commit-ID: c02f48e070bde326f55bd94544ca82291f7396e3 Gitweb: https://git.kernel.org/tip/c02f48e070bde326f55bd94544ca82291f7396e3 Author: Borislav Petkov <bp@suse.de> AuthorDate: Fri, 5 Apr 2019 06:28:11 +0200 Committer: Borislav Petkov <bp@suse.de> CommitDate: Wed, 10 Apr 2019 22:43:24 +0200 x86/microcode: Deprecate MICROCODE_OLD_INTERFACE This is the ancient loading interface which requires special tools to be used. The bigger problem with it is that it is as inadequate for proper loading of microcode as the late microcode loading interface is because it happens just as late. iucode_tool's manpage already warns people that it is deprecated. Deprecate it and turn it off by default along with a big fat warning in the Kconfig help. It will go away sometime in the future. Signed-off-by: Borislav Petkov <bp@suse.de> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Cc: x86@kernel.org Link: https://lkml.kernel.org/r/20190405133010.24249-4-bp@alien8.de --- arch/x86/Kconfig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 5ad92419be19..5a0a752f3ddd 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1330,8 +1330,16 @@ config MICROCODE_AMD processors will be enabled. config MICROCODE_OLD_INTERFACE - def_bool y + bool "Ancient loading interface (DEPRECATED)" + default n depends on MICROCODE + ---help--- + DO NOT USE THIS! This is the ancient /dev/cpu/microcode interface + which was used by userspace tools like iucode_tool and microcode.ctl. + It is inadequate because it runs too late to be able to properly + load microcode on a machine and it needs special tools. Instead, you + should've switched to the early loading method with the initrd or + builtin microcode by now: Documentation/x86/microcode.txt config X86_MSR tristate "/dev/cpu/*/msr - Model-specific register support" ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-04-10 20:50 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-05 13:30 [PATCH 0/3] x86/microcode: Some fixes Borislav Petkov 2019-04-05 13:30 ` [PATCH 1/3] x86/microcode/intel: Refactor Intel microcode blob loading Borislav Petkov 2019-04-05 21:59 ` Thomas Gleixner 2019-04-05 13:30 ` [PATCH 2/3] x86/microcode: Fix the ancient deprecated microcode loading method Borislav Petkov 2019-04-05 21:55 ` Thomas Gleixner 2019-04-10 20:49 ` [tip:x86/microcode] " tip-bot for Borislav Petkov 2019-04-05 13:30 ` [PATCH 3/3] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE Borislav Petkov 2019-04-05 21:53 ` Thomas Gleixner 2019-04-10 20:50 ` [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).