Messages in this thread Patch in this message |  | | Date | Fri, 26 Jan 2018 17:23:31 +0100 | From | Andrea Arcangeli <> | Subject | Re: [patch V2 1/2] sysfs/cpu: Add vulnerability folder |
| |
Hello,
On Sun, Jan 07, 2018 at 10:48:00PM +0100, Thomas Gleixner wrote: > +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);
This sysfs feature implemented as above is weakening kernel security, it should be 0400 above.
It doesn't make sense to expose to luser when a software fix (or even only a software mitigation) has been disabled at build time to gain all performance back (see CONFIG_RETPOLINE=n config option).
$ cat /boot/kernel-`uname -r` cat: /boot/kernel-4.15.0-rc9+: Permission denied $ cat /lib/modules/`uname -r`/kernel/arch/x86/kvm/kvm.ko cat: /lib/modules/4.15.0-rc9+/kernel/arch/x86/kvm/kvm.ko: Permission denied $ dmesg dmesg: read kernel buffer failed: Operation not permitted
Noticing when cr3 is flipped in kernel/exit is easy, but noticing when the syscall table or the whole kernel has been built with retpolines is not trivial to detect. Same for variant#1.
Exploiting spectre variant#2 for an attacker is not without risk of being detected while the setup is being mounted, as the CPU load would spike for hours.
I may notice if a random background network daemon suddenly starts running at 100% CPU load for hours (especially on mobile devices that would be physically noticeable).
Containers shouldn't have sysfs and you can workaround the above if you run all network daemons behind mount namespaces, but in general leaving this directory readable by luser is weaking security because it exposes when mounting a variant#2 attack can succeed.
It even tells when it is worth to focus on the syscall_table indirect call or if the attack needs to dig deeper because asm retpolines were used, but the kernel was built with a gcc without retpolines.
The only case where the above isn't weakening security is when the full fix is on for all the variants is enabled (and variant#1 for now just shows vulnerable..).
For the same reasons the whole directory, not just the files, should be 0500, especially if this would be used for any other equivalent issue in the future and it won't stick to these 3 files, I didn't implement that yet, because it's less urgent if nobody adds any more files soon.
From 578b411c8dcb1435dd1f94a6cd062f4eedb70fb5 Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli <aarcange@redhat.com> Date: Wed, 24 Jan 2018 19:19:36 +0100 Subject: [PATCH 1/1] x86/spectre/meltdown: avoid the vulnerability directory to weaken kernel security
If any of the fixes is disabled to gain some performance back at runtime or build time, should not be exposed to unprivileged userland.
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com> --- 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,
|  |