[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPcyv4gA5eBYGsb+5WGcCnyB-8MB6qeO9Sw6aZNvaBCtb7OK1w@mail.gmail.com>
Date: Mon, 29 Jan 2018 14:05:51 -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 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"
...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.
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.
