From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Williams Subject: Re: [PATCH v5 12/12] x86/spectre: report get_user mitigation for spectre_v1 Date: Mon, 29 Jan 2018 14:05:51 -0800 Message-ID: References: <151703971300.26578.1185595719337719486.stgit@dwillia2-desk3.amr.corp.intel.com> <151703977742.26578.8362387033092864423.stgit@dwillia2-desk3.amr.corp.intel.com> <20180128095027.hqrcpnholsvukdzd@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: Received: from mail-oi0-f66.google.com ([209.85.218.66]:42359 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751837AbeA2WFx (ORCPT ); Mon, 29 Jan 2018 17:05:53 -0500 Received: by mail-oi0-f66.google.com with SMTP id c8so6266666oiy.9 for ; Mon, 29 Jan 2018 14:05:52 -0800 (PST) In-Reply-To: <20180128095027.hqrcpnholsvukdzd@gmail.com> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Ingo Molnar Cc: Thomas Gleixner , linux-arch , Kernel Hardening , Greg KH , X86 ML , Linus Torvalds , Ingo Molnar , "H. Peter Anvin" , Jiri Slaby , Alan Cox On Sun, Jan 28, 2018 at 1:50 AM, Ingo Molnar wrote: > > * Dan Williams wrote: > >> Reflect the presence of 'get_user', '__get_user', and 'syscall' >> protections in sysfs. Keep the "Vulnerable" distinction given the >> expectation that the places that have been identified for 'array_idx' >> usage are likely incomplete. > > (The style problems/inconsistencies of the previous patches are repeated here too, > please fix.) > >> >> Cc: Thomas Gleixner >> Cc: Ingo Molnar >> Cc: "H. Peter Anvin" >> Cc: x86@kernel.org >> Cc: Greg Kroah-Hartman >> Reported-by: Jiri Slaby >> Signed-off-by: Dan Williams >> --- >> arch/x86/kernel/cpu/bugs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c >> index 390b3dc3d438..01d5ba48f745 100644 >> --- a/arch/x86/kernel/cpu/bugs.c >> +++ b/arch/x86/kernel/cpu/bugs.c >> @@ -269,7 +269,7 @@ ssize_t cpu_show_spectre_v1(struct device *dev, >> { >> if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V1)) >> return sprintf(buf, "Not affected\n"); >> - return sprintf(buf, "Vulnerable\n"); >> + return sprintf(buf, "Vulnerable: Minimal user pointer sanitization\n"); > > Btw., I think this string is still somewhat passive-aggressive towards users, as > it doesn't really give them any idea about what is missing from their system so > that they can turn it into not vulnerable. > > What else is missing that would turn this into a "Mitigated" entry? Part of the problem is that there are different sub-classes of Spectre variant1 vulnerabilities. For example, speculating on the value of a user pointer returned from get_user() is mitigated by these kernel changes. However, cleaning up occasions where the CPU might speculate on the validity of a user-controlled pointer offset, or user-controlled array index is only covered by manual inspection of some noisy / incomplete tooling results. I.e. the handful of array_index_nospec() usages in this series is likely incomplete. The usage of barrier_nospec() in __get_user() and open coded array_index_nospec() in get_user() does raise the bar and mitigates an entire class of problems. Perhaps it would be reasonable to have cpu_show_spectre_v1() emit: "Mitigation: __user pointer sanitization" ...with the expectation that the kernel community intends to use new and better tooling to find more places to use array_index_nospec(). Once there is wider confidence in that tooling, or a compiler that does it automatically, the kernel can emit: "Mitigation: automated user input sanitization" ...or something like that.