diff options
| author | Dave Hansen <dave.hansen@linux.intel.com> | 2023-02-21 12:30:15 -0800 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2023-02-25 11:53:26 +0100 |
| commit | 6c750ed0367f6bf1b09c0c353a701781ee05dd22 (patch) | |
| tree | fc8ef2c563a30fb3c9c64ec0078ad29b91838270 | |
| parent | 4d2e5de071fd5da69c3561ef360c8898143fc56a (diff) | |
| download | linux-6c750ed0367f6bf1b09c0c353a701781ee05dd22.tar.gz linux-6c750ed0367f6bf1b09c0c353a701781ee05dd22.tar.bz2 linux-6c750ed0367f6bf1b09c0c353a701781ee05dd22.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.h | 4 | ||||
| -rw-r--r-- | kernel/bpf/core.c | 2 | ||||
| -rw-r--r-- | lib/usercopy.c | 7 |
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 6b33a8a148b8..556d67afc493 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -1567,9 +1567,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 cbb4d9ec00f2..82881e0bcaa1 100644 --- a/lib/usercopy.c +++ b/lib/usercopy.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include <linux/uaccess.h> #include <linux/bitops.h> +#include <linux/nospec.h> /* out-of-line parts */ @@ -10,6 +11,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(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); } |
