summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDave Hansen <dave.hansen@linux.intel.com>2023-02-21 12:30:15 -0800
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2023-02-25 11:51:51 +0100
commitf8e54da1c729cc23d9a7b7bd42379323e7fb7979 (patch)
tree46942a1abe8f56c13e24495949f63ed78a5d8093
parente936e3fd614f0b262780a762c2d6008d6c672991 (diff)
downloadlinux-stable-f8e54da1c729cc23d9a7b7bd42379323e7fb7979.tar.gz
linux-stable-f8e54da1c729cc23d9a7b7bd42379323e7fb7979.tar.bz2
linux-stable-f8e54da1c729cc23d9a7b7bd42379323e7fb7979.zip
uaccess: Add speculation barrier to copy_from_user()
commit 74e19ef0ff8061ef55957c3abd71614ef0f42f47 upstream. The results of "access_ok()" can be mis-speculated. The result is that you can end speculatively: if (access_ok(from, size)) // Right here even for bad from/size combinations. On first glance, it would be ideal to just add a speculation barrier to "access_ok()" so that its results can never be mis-speculated. But there are lots of system calls just doing access_ok() via "copy_to_user()" and friends (example: fstat() and friends). Those are generally not problematic because they do not _consume_ data from userspace other than the pointer. They are also very quick and common system calls that should not be needlessly slowed down. "copy_from_user()" on the other hand uses a user-controller pointer and is frequently followed up with code that might affect caches. Take something like this: if (!copy_from_user(&kernelvar, uptr, size)) do_something_with(kernelvar); If userspace passes in an evil 'uptr' that *actually* points to a kernel addresses, and then do_something_with() has cache (or other) side-effects, it could allow userspace to infer kernel data values. Add a barrier to the common copy_from_user() code to prevent mis-speculated values which happen after the copy. Also add a stub for architectures that do not define barrier_nospec(). This makes the macro usable in generic code. Since the barrier is now usable in generic code, the x86 #ifdef in the BPF code can also go away. Reported-by: Jordy Zomer <jordyzomer@google.com> Suggested-by: Linus Torvalds <torvalds@linuxfoundation.org> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Acked-by: Daniel Borkmann <daniel@iogearbox.net> # BPF bits Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--include/linux/nospec.h4
-rw-r--r--kernel/bpf/core.c2
-rw-r--r--lib/usercopy.c7
3 files changed, 11 insertions, 2 deletions
diff --git a/include/linux/nospec.h b/include/linux/nospec.h
index 0c5ef54fd416..207ef2a20e48 100644
--- a/include/linux/nospec.h
+++ b/include/linux/nospec.h
@@ -9,6 +9,10 @@
struct task_struct;
+#ifndef barrier_nospec
+# define barrier_nospec() do { } while (0)
+#endif
+
/**
* array_index_mask_nospec() - generate a ~0 mask when index < size, 0 otherwise
* @index: array element index
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index cbbd0168f50c..067d504d2841 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1373,9 +1373,7 @@ out:
* reuse preexisting logic from Spectre v1 mitigation that
* happens to produce the required code on x86 for v4 as well.
*/
-#ifdef CONFIG_X86
barrier_nospec();
-#endif
CONT;
#define LDST(SIZEOP, SIZE) \
STX_MEM_##SIZEOP: \
diff --git a/lib/usercopy.c b/lib/usercopy.c
index 3744b2a8e591..1e99c1baf4ff 100644
--- a/lib/usercopy.c
+++ b/lib/usercopy.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/uaccess.h>
+#include <linux/nospec.h>
/* out-of-line parts */
@@ -9,6 +10,12 @@ unsigned long _copy_from_user(void *to, const void __user *from, unsigned long n
unsigned long res = n;
might_fault();
if (likely(access_ok(VERIFY_READ, from, n))) {
+ /*
+ * Ensure that bad access_ok() speculation will not
+ * lead to nasty side effects *after* the copy is
+ * finished:
+ */
+ barrier_nospec();
kasan_check_write(to, n);
res = raw_copy_from_user(to, from, n);
}