2018-01-25 12:05:51

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH] cpu: do not leak vulnerabilities to unprivileged users

While it's public information if the CPU in general has spectre/meltdown
bugs, it probably shouldn't be as globally obvious to all unprivileged
users whether or not the kernel is doing something to mitigate those
bugs. While an attacker can obviously probe and try, there frequently is
a trade-off attackers make of how much probing around they're willing to
do versus the certainty of an attack working, in order to reduce
detection. By making it loud and clear that the kernel _is_ vulnerable,
we're simply aiding the trade-off calculations attackers have to make
when choosing which vectors to target.

So, this patch changes the permissions to 0400 to make the attacker's
job slightly less easy.

Signed-off-by: Jason A. Donenfeld <[email protected]>
---
drivers/base/cpu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index d99038487a0d..a3a8e008f957 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -531,9 +531,9 @@ ssize_t __weak cpu_show_spectre_v2(struct device *dev,
return sprintf(buf, "Not affected\n");
}

-static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL);
-static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL);
-static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);
+static DEVICE_ATTR(meltdown, 0400, cpu_show_meltdown, NULL);
+static DEVICE_ATTR(spectre_v1, 0400, cpu_show_spectre_v1, NULL);
+static DEVICE_ATTR(spectre_v2, 0400, cpu_show_spectre_v2, NULL);

static struct attribute *cpu_root_vulnerabilities_attrs[] = {
&dev_attr_meltdown.attr,
--
2.16.1



2018-01-25 13:40:50

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [kernel-hardening] Re: [PATCH] cpu: do not leak vulnerabilities to unprivileged users

On Thu, Jan 25, 2018 at 2:34 PM, Alan Cox <[email protected]> wrote:
> As you observe any attacker can already trivially ascertain whether
> protection is on, so there is no point pretending file permissions
> magically stop that. In fact the information is already in cpuinfo.

Actually the other place it leaks is in dmesg, which would need to be
patched too.

My understanding about cpuinfo was that it showed whether or not the
processor family is generally vulnerable to it, independent of whether
or not the kernel has been patched. What this patch does relates to
whether or not the kernel has been patched.

2018-01-25 14:34:23

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH] cpu: do not leak vulnerabilities to unprivileged users

On Thu, 25 Jan 2018 13:04:01 +0100
"Jason A. Donenfeld" <[email protected]> wrote:

> While it's public information if the CPU in general has spectre/meltdown
> bugs, it probably shouldn't be as globally obvious to all unprivileged
> users whether or not the kernel is doing something to mitigate

There are plenty of cases where it is useful for an application such as a
JIT to know what level of protection it needs to be providing. For
example if you look across the ecosystem (notably ARM) a lot of common
slower processors are not vulnerable. For those a JIT would want to
generate code without the overhead of any protections.

As you observe any attacker can already trivially ascertain whether
protection is on, so there is no point pretending file permissions
magically stop that. In fact the information is already in cpuinfo.

IMHO given it's trivially available info and useful for JITs it make
sense for the data to be exposed.

Alan

2018-01-26 12:33:19

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH v2] cpu: do not leak vulnerabilities to unprivileged users

While it's public information if the CPU in general has spectre/meltdown
bugs, it probably shouldn't be as globally obvious to all unprivileged
users whether or not the kernel is doing something to mitigate those
bugs. While an attacker can obviously probe and try, there frequently is
a trade-off attackers make of how much probing around they're willing to
do versus the certainty of an attack working, in order to reduce
detection. By making it loud and clear that the kernel _is_ vulnerable,
we're simply aiding the trade-off calculations attackers have to make
when choosing which vectors to target.

So, this patch changes the permissions to 0400 to make the attacker's
job slightly less easy. While we're at it, we clean up the leak in dmesg
too.

Signed-off-by: Jason A. Donenfeld <[email protected]>
---
v2 handles dmesg too.

arch/x86/kernel/cpu/bugs.c | 1 -
drivers/base/cpu.c | 6 +++---
2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 390b3dc3d438..e512ae82f201 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -230,7 +230,6 @@ static void __init spectre_v2_select_mitigation(void)
}

spectre_v2_enabled = mode;
- pr_info("%s\n", spectre_v2_strings[mode]);

/*
* If neither SMEP or KPTI are available, there is a risk of
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index d99038487a0d..a3a8e008f957 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -531,9 +531,9 @@ ssize_t __weak cpu_show_spectre_v2(struct device *dev,
return sprintf(buf, "Not affected\n");
}

-static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL);
-static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL);
-static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);
+static DEVICE_ATTR(meltdown, 0400, cpu_show_meltdown, NULL);
+static DEVICE_ATTR(spectre_v1, 0400, cpu_show_spectre_v1, NULL);
+static DEVICE_ATTR(spectre_v2, 0400, cpu_show_spectre_v2, NULL);

static struct attribute *cpu_root_vulnerabilities_attrs[] = {
&dev_attr_meltdown.attr,
--
2.16.1


2018-01-26 16:44:13

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v2] cpu: do not leak vulnerabilities to unprivileged users

On Fri, 26 Jan 2018 13:31:58 +0100
"Jason A. Donenfeld" <[email protected]> wrote:

> While it's public information if the CPU in general has spectre/meltdown
> bugs, it probably shouldn't be as globally obvious to all unprivileged
> users

As I replied to you last time you posted this

a) The info is already trivially accessible via /proc/cpuinfo or by
measurement to an attacker
b) Some JIT and other environments need to know

Alan

2018-01-26 17:23:57

by Boris Lukashev

[permalink] [raw]
Subject: Re: [kernel-hardening] Re: [PATCH v2] cpu: do not leak vulnerabilities to unprivileged users

This sort of seems like painting a plate carrier onto a tshirt.
Attackers will know, one way or another, as we have illegitimate and
legitimate vectors for gaining information. Good ecosystem inhabitants
however will be in the dark because this proposes to obfuscate their
legitimate interfaces used to determine execution context.
This might be a better option for namespaces - run an NS with
"hardware spectre mitigation masking" when the consumers in it can
handle that, but leave the rest of the userspace only as broken as it
was before. Without additional protections, bad guys can still glean
the info via leaks and functional testing, but it should provide a
small hurdle without sacrificing JIT ops in the primary NS.

-Boris

On Fri, Jan 26, 2018 at 11:43 AM, Alan Cox <[email protected]> wrote:
> On Fri, 26 Jan 2018 13:31:58 +0100
> "Jason A. Donenfeld" <[email protected]> wrote:
>
>> While it's public information if the CPU in general has spectre/meltdown
>> bugs, it probably shouldn't be as globally obvious to all unprivileged
>> users
>
> As I replied to you last time you posted this
>
> a) The info is already trivially accessible via /proc/cpuinfo or by
> measurement to an attacker
> b) Some JIT and other environments need to know
>
> Alan



--
Boris Lukashev
Systems Architect
Semper Victus

2018-01-26 17:48:20

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [kernel-hardening] Re: [PATCH v2] cpu: do not leak vulnerabilities to unprivileged users

On Fri, Jan 26, 2018 at 5:43 PM, Alan Cox <[email protected]> wrote:
> a) The info is already trivially accessible via /proc/cpuinfo

No, /proc/cpuinfo shows if the CPU itself has these bugs, but doesn't
show whether or not the kernel has gone to lengths to mitigate these
bugs.

# grep -o 'bugs.*cpu_meltdown' -m1 /proc/cpuinfo
bugs : cpu_meltdown
# cat /sys/devices/system/cpu/vulnerabilities/meltdown
Mitigation: PTI

> or by measurement to an attacker

Right, so without this, an attacker has to measure. The purpose of
this patchset is to require the attacker to perform an additional
measurement. That seems worthwhile, especially if measurements are or
would ever become non-trivial to make.

> b) Some JIT and other environments need to know

Shouldn't JITs do the best they can with the environment they're in?
And for that, isn't /proc/cpuinfo enough?

Jason

2018-01-26 18:47:02

by Boris Lukashev

[permalink] [raw]
Subject: Re: [kernel-hardening] Re: [PATCH v2] cpu: do not leak vulnerabilities to unprivileged users

On Fri, Jan 26, 2018 at 12:47 PM, Jason A. Donenfeld <[email protected]> wrote:
> On Fri, Jan 26, 2018 at 5:43 PM, Alan Cox <[email protected]> wrote:
>> a) The info is already trivially accessible via /proc/cpuinfo
>
> No, /proc/cpuinfo shows if the CPU itself has these bugs, but doesn't
> show whether or not the kernel has gone to lengths to mitigate these
> bugs.
>
> # grep -o 'bugs.*cpu_meltdown' -m1 /proc/cpuinfo
> bugs : cpu_meltdown
> # cat /sys/devices/system/cpu/vulnerabilities/meltdown
> Mitigation: PTI
>
>> or by measurement to an attacker
>
> Right, so without this, an attacker has to measure. The purpose of
> this patchset is to require the attacker to perform an additional
> measurement. That seems worthwhile, especially if measurements are or
> would ever become non-trivial to make.
>
>> b) Some JIT and other environments need to know
>
> Shouldn't JITs do the best they can with the environment they're in?

In an ideal world, anything using JIT would have a graceful fail-down
to pre-compiled generic paths. As we've seen with PaX' MPROTECT
killing half of the desktop environment along with third party
(browsers/editors/etc) applications, and pretty much every interpreter
or FFI binding-compiled-binary; there's a lot of JIT
compilation/execution out there which doesnt take failure well. I'm
not saying that this would be as detrimental, but in cases where the
JIT gets confused and does "the wrong thing," all sorts of undefined
behaviors might occur ranging from poorly performing execution to
security concerns and crashes. _If_ such confusion is possible, is the
benefit of a layer of concealment (as opposed to cover) worth the
potential effort in tracking down problems which could be caused by
this? How likely are users to bypass the constraint by running as root
and expose their system to more threat?

> And for that, isn't /proc/cpuinfo enough?
>
> Jason



--
Boris Lukashev
Systems Architect
Semper Victus

2018-01-26 19:12:46

by Alan Cox

[permalink] [raw]
Subject: Re: [kernel-hardening] Re: [PATCH v2] cpu: do not leak vulnerabilities to unprivileged users

On Fri, 26 Jan 2018 18:47:28 +0100
"Jason A. Donenfeld" <[email protected]> wrote:

> On Fri, Jan 26, 2018 at 5:43 PM, Alan Cox <[email protected]> wrote:
> > a) The info is already trivially accessible via /proc/cpuinfo
>
> No, /proc/cpuinfo shows if the CPU itself has these bugs, but doesn't
> show whether or not the kernel has gone to lengths to mitigate these
> bugs.

It tells you immediately what microcode, what hardware properties. A few
user space instructions later and you know the rest.

> Right, so without this, an attacker has to measure. The purpose of
> this patchset is to require the attacker to perform an additional
> measurement. That seems worthwhile, especially if measurements are or
> would ever become non-trivial to make.

You mean 'I'm magically going to make it secure because the attacker
won't be smart enough to paste in a function from gitub' ?

I don't buy it. Most attack tools are automated, they will contain the
needed code.

> > b) Some JIT and other environments need to know
>
> Shouldn't JITs do the best they can with the environment they're in?
> And for that, isn't /proc/cpuinfo enough?

No

The JIT needs to know whether the mitigations are present because it
needs to JIT very different code if it is responsible for generating
things like lfence and and/mask references or can assume the CPU is
covering some or part of it.

Likewise you can imagine other code using those files in order to figure
out how to self patch / which library to load for performance critical
stuff.

Alan