x86/microcode: Some fixes
Subsystems (2)
Borislav Petkov
maintainercover x86/microcode: Some fixes
From: Borislav Petkov <[email protected]>
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(-)
Signature · 1 line
2.21.0
Borislav Petkov
maintainerPATCH 1/3 x86/microcode/intel: Refactor Intel microcode blob loading
From: Jann Horn <[email protected]>
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. ]
--- arch/x86/kernel/cpu/microcode/intel.c | 71 ++++++++++++++------------- 1 file changed, 36 insertions(+), 35 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +36 -35
@@ -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 = {
Signature · 1 line
2.21.0
Re: [PATCH 1/3] x86/microcode/intel: Refactor Intel microcode blob loading
Quoted 7 lines · Borislav Petkov
From: Jann Horn <[email protected]>
Change
generic_load_microcode()to use theiov_iterAPI instead of a clumsy open-coded version which has to pay attention to__userdata or kernel data, depending on the loading method. This allows to avoid explicit casting between user and kernel pointers.
Quoted 6 lines
Because the
iov_iterAPI 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 inmicrocode_sanity_check()).Not that it matters much, only root is allowed to load microcode anyway...
Nice!
Borislav Petkov
maintainerPATCH 2/3 x86/microcode: Fix the ancient deprecated microcode loading method
In reply to Borislav PetkovFrom: Borislav Petkov <[email protected]>
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.
--- arch/x86/kernel/cpu/microcode/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/microcode/core.c +2 -1
@@ -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;
Signature · 1 line
2.21.0
Re: [PATCH 2/3] x86/microcode: Fix the ancient deprecated microcode loading method
Quoted 3 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
The commit in Fixes: added the new define
UCODE_NEWto denote that an
That reads odd. What's wrong with:
Commit 2613f36ed965 added ....
or
A recent commit added
mm?
Quoted 9 lines
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/microcodeloading interface. Fix it.Fixes 2613f36ed965 ("x86/microcode: Attempt late loading only when new microcode is present")Signed-off-by Borislav Petkov <[email protected]>Cc Jann Horn <[email protected]>
Other than that:
Quoted 23 lines
--- arch/x86/kernel/cpu/microcode/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/x86/kernel/cpu/microcode/core.c +2
-1@@ -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;Signature · 1 line
2.21.0
tip-bot for Borislav Petkov
merge bot[tip:x86/microcode] x86/microcode: Fix the ancient deprecated microcode loading method
In reply to Borislav Petkovx86/microcode: Fix the ancient deprecated microcode loading method
Commit-ID: 24613a04ad1c0588c10f4b5403ca60a73d164051 Gitweb: https://git.kernel.org/tip/24613a04ad1c0588c10f4b5403ca60a73d164051 Author: Borislav Petkov <[email protected]> AuthorDate: Thu, 4 Apr 2019 22:14:07 +0200 Committer: Borislav Petkov <[email protected]> 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.
--- arch/x86/kernel/cpu/microcode/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/microcode/core.c +2 -1
@@ -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;
Borislav Petkov
maintainerPATCH 3/3 x86/microcode: Deprecate MICROCODE_OLD_INTERFACE
In reply to Borislav PetkovFrom: Borislav Petkov <[email protected]>
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.
--- arch/x86/Kconfig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
arch/x86/Kconfig +9 -1
@@ -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"
Signature · 1 line
2.21.0
Re: [PATCH 3/3] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE
Quoted 7 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
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.
Quoted 6 lines
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
Kconfighelp. It will go away sometime in the future.Signed-off-by Borislav Petkov <[email protected]>
tip-bot for Borislav Petkov
merge bot[tip:x86/microcode] x86/microcode: Deprecate MICROCODE_OLD_INTERFACE
In reply to Borislav Petkovx86/microcode: Deprecate MICROCODE_OLD_INTERFACE
Commit-ID: c02f48e070bde326f55bd94544ca82291f7396e3 Gitweb: https://git.kernel.org/tip/c02f48e070bde326f55bd94544ca82291f7396e3 Author: Borislav Petkov <[email protected]> AuthorDate: Fri, 5 Apr 2019 06:28:11 +0200 Committer: Borislav Petkov <[email protected]> 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.
--- arch/x86/Kconfig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
arch/x86/Kconfig +9 -1
@@ -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"