* Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> On Sun, Jan 28, 2018 at 1:50 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >
> > * Dan Williams <dan.j.williams@xxxxxxxxx> 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@xxxxxxxxxxxxx>
> >> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> >> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> >> Cc: x86@xxxxxxxxxx
> >> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> >> Reported-by: Jiri Slaby <jslaby@xxxxxxx>
> >> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> >> ---
> >> 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
