[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAPcyv4jdDkwGFW1oqdL_wDS1b55ryB5OjDb3PmERsMZ0mz=z=g@mail.gmail.com>
Date: Thu, 1 Feb 2018 12:23:01 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Ingo Molnar <mingo@...nel.org>
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
On Wed, Jan 31, 2018 at 12:07 AM, Ingo Molnar <mingo@...nel.org> wrote:
>
> * 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.
But that would assume that it's only array index sanitization that we
need to worry about. The get_user() protections were interesting
because they showed a class of potential leaks near pointer
de-references not necessarily arrays. So I think it's more likely the
case that we'll add more "Mitigation:" lines over time.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
