Skip to content

Commit 74e19ef

Browse files
hansendctorvalds
authored andcommitted
uaccess: Add speculation barrier to copy_from_user()
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 <[email protected]> Suggested-by: Linus Torvalds <[email protected]> Signed-off-by: Dave Hansen <[email protected]> Reviewed-by: Thomas Gleixner <[email protected]> Acked-by: Daniel Borkmann <[email protected]> # BPF bits Signed-off-by: Linus Torvalds <[email protected]>
1 parent 1b72607 commit 74e19ef

File tree

3 files changed

+11
-2
lines changed

3 files changed

+11
-2
lines changed

include/linux/nospec.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@
1111

1212
struct task_struct;
1313

14+
#ifndef barrier_nospec
15+
# define barrier_nospec() do { } while (0)
16+
#endif
17+
1418
/**
1519
* array_index_mask_nospec() - generate a ~0 mask when index < size, 0 otherwise
1620
* @index: array element index

kernel/bpf/core.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1910,9 +1910,7 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
19101910
* reuse preexisting logic from Spectre v1 mitigation that
19111911
* happens to produce the required code on x86 for v4 as well.
19121912
*/
1913-
#ifdef CONFIG_X86
19141913
barrier_nospec();
1915-
#endif
19161914
CONT;
19171915
#define LDST(SIZEOP, SIZE) \
19181916
STX_MEM_##SIZEOP: \

lib/usercopy.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <linux/fault-inject-usercopy.h>
44
#include <linux/instrumented.h>
55
#include <linux/uaccess.h>
6+
#include <linux/nospec.h>
67

78
/* out-of-line parts */
89

@@ -12,6 +13,12 @@ unsigned long _copy_from_user(void *to, const void __user *from, unsigned long n
1213
unsigned long res = n;
1314
might_fault();
1415
if (!should_fail_usercopy() && likely(access_ok(from, n))) {
16+
/*
17+
* Ensure that bad access_ok() speculation will not
18+
* lead to nasty side effects *after* the copy is
19+
* finished:
20+
*/
21+
barrier_nospec();
1522
instrument_copy_from_user_before(to, from, n);
1623
res = raw_copy_from_user(to, from, n);
1724
instrument_copy_from_user_after(to, from, n, res);

0 commit comments

Comments
 (0)