kernel-hardening - Re: [PATCH v5 12/12] x86/spectre: report get_user mitigation for spectre

Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180131080714.svyq34reb7d2pwmf@gmail.com>
Date: Wed, 31 Jan 2018 09:07:15 +0100
From: Ingo Molnar <mingo@...nel.org>
To: Dan Williams <dan.j.williams@...el.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
	linux-arch <linux-arch@...r.kernel.org>,
	Kernel Hardening <kernel-hardening@...ts.openwall.com>,
	Greg KH <gregkh@...uxfoundation.org>, X86 ML <x86@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
	Jiri Slaby <jslaby@...e.cz>, Alan Cox <alan@...ux.intel.com>
Subject: Re: [PATCH v5 12/12] x86/spectre: report get_user mitigation for
 spectre_v1


* Dan Williams <dan.j.williams@...el.com> wrote:

> On Sun, Jan 28, 2018 at 1:50 AM, Ingo Molnar <mingo@...nel.org> wrote:
> >
> > * Dan Williams <dan.j.williams@...el.com> 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 <tglx@...utronix.de>
> >> Cc: Ingo Molnar <mingo@...hat.com>
> >> Cc: "H. Peter Anvin" <hpa@...or.com>
> >> Cc: x86@...nel.org
> >> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> >> Reported-by: Jiri Slaby <jslaby@...e.cz>
> >> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> >> ---
> >>  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"

Yeah, so I think the ideal approach would be if we emitted _both_ pieces of 
information:

     Mitigation: __user pointer sanitization
     Vulnerable: incomplete array index sanitization

I.e. we inform the user about the mitigation measures that are active, but we also 
fully inform that we think it's incomplete at this stage.

> ...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.

Yeah.

Thanks,

	Ingo
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.