Messages in this thread | | | Date | Tue, 31 Jan 2023 20:17:25 +0800 | Subject | Re: [Patch v3 Part2 1/9] x86/microcode: Taint kernel only if microcode loading was successful | From | "Li, Aubrey" <> |
| |
On 2023/1/31 5:39, Ashok Raj wrote: > Currently when late loading is aborted due to check_online_cpu(), kernel > still ends up tainting the kernel. > > Taint only when microcode loading was successful. > > Suggested-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Ashok Raj <ashok.raj@intel.com> > Cc: LKML <linux-kernel@vger.kernel.org> > Cc: x86 <x86@kernel.org> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Tony Luck <tony.luck@intel.com> > Cc: Dave Hansen <dave.hansen@intel.com> > Cc: Alison Schofield <alison.schofield@intel.com> > Cc: Reinette Chatre <reinette.chatre@intel.com> > Cc: Thomas Gleixner (Intel) <tglx@linutronix.de> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: Stefan Talpalaru <stefantalpalaru@yahoo.com> > Cc: David Woodhouse <dwmw2@infradead.org> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: Rafael J. Wysocki <rafael@kernel.org> > Cc: Peter Zilstra (Intel) <peterz@infradead.org> > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > Cc: Martin Pohlack <mpohlack@amazon.de> > --- > v1->v2: (Thomas) > - Remove unnecessary assignment of ret that's being overwritten. > - Taint kernel only of loading was successful > --- > arch/x86/kernel/cpu/microcode/core.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c > index 61d57d9b93ee..1c6831b8b244 100644 > --- a/arch/x86/kernel/cpu/microcode/core.c > +++ b/arch/x86/kernel/cpu/microcode/core.c > @@ -472,7 +472,8 @@ static ssize_t reload_store(struct device *dev, > enum ucode_state tmp_ret = UCODE_OK; > int bsp = boot_cpu_data.cpu_index; > unsigned long val; > - ssize_t ret = 0; > + int load_ret = -1; > + ssize_t ret; > > ret = kstrtoul(buf, 0, &val); > if (ret) > @@ -488,20 +489,26 @@ static ssize_t reload_store(struct device *dev, > goto put; > > tmp_ret = microcode_ops->request_microcode_fw(bsp, µcode_pdev->dev); > - if (tmp_ret != UCODE_NEW) > + if (tmp_ret != UCODE_NEW) { > + ret = size; > goto put; > + } > > mutex_lock(µcode_mutex); > - ret = microcode_reload_late(); > + load_ret = microcode_reload_late(); > mutex_unlock(µcode_mutex); > > put: > cpus_read_unlock(); > > - if (ret == 0) > + /* > + * Taint only when loading was successful > + */ > + if (load_ret == 0) { > ret = size;
What about if loading was not successful(load_ret != 0)? ret has no chance to be returned as size here and we'll run into the endless update?
Thanks, -Aubrey
> - > - add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK); > + add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK); > + pr_warn("Microcode late loading tainted the kernel\n"); > + } > > return ret; > }
| |