mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	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 <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>
This commit is contained in:
		
							parent
							
								
									1b72607d73
								
							
						
					
					
						commit
						74e19ef0ff
					
				
					 3 changed files with 11 additions and 2 deletions
				
			
		| 
						 | 
					@ -11,6 +11,10 @@
 | 
				
			||||||
 | 
					
 | 
				
			||||||
struct task_struct;
 | 
					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
 | 
					 * array_index_mask_nospec() - generate a ~0 mask when index < size, 0 otherwise
 | 
				
			||||||
 * @index: array element index
 | 
					 * @index: array element index
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1910,9 +1910,7 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
 | 
				
			||||||
		 * reuse preexisting logic from Spectre v1 mitigation that
 | 
							 * reuse preexisting logic from Spectre v1 mitigation that
 | 
				
			||||||
		 * happens to produce the required code on x86 for v4 as well.
 | 
							 * happens to produce the required code on x86 for v4 as well.
 | 
				
			||||||
		 */
 | 
							 */
 | 
				
			||||||
#ifdef CONFIG_X86
 | 
					 | 
				
			||||||
		barrier_nospec();
 | 
							barrier_nospec();
 | 
				
			||||||
#endif
 | 
					 | 
				
			||||||
		CONT;
 | 
							CONT;
 | 
				
			||||||
#define LDST(SIZEOP, SIZE)						\
 | 
					#define LDST(SIZEOP, SIZE)						\
 | 
				
			||||||
	STX_MEM_##SIZEOP:						\
 | 
						STX_MEM_##SIZEOP:						\
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -3,6 +3,7 @@
 | 
				
			||||||
#include <linux/fault-inject-usercopy.h>
 | 
					#include <linux/fault-inject-usercopy.h>
 | 
				
			||||||
#include <linux/instrumented.h>
 | 
					#include <linux/instrumented.h>
 | 
				
			||||||
#include <linux/uaccess.h>
 | 
					#include <linux/uaccess.h>
 | 
				
			||||||
 | 
					#include <linux/nospec.h>
 | 
				
			||||||
 | 
					
 | 
				
			||||||
/* out-of-line parts */
 | 
					/* out-of-line parts */
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -12,6 +13,12 @@ unsigned long _copy_from_user(void *to, const void __user *from, unsigned long n
 | 
				
			||||||
	unsigned long res = n;
 | 
						unsigned long res = n;
 | 
				
			||||||
	might_fault();
 | 
						might_fault();
 | 
				
			||||||
	if (!should_fail_usercopy() && likely(access_ok(from, n))) {
 | 
						if (!should_fail_usercopy() && 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();
 | 
				
			||||||
		instrument_copy_from_user_before(to, from, n);
 | 
							instrument_copy_from_user_before(to, from, n);
 | 
				
			||||||
		res = raw_copy_from_user(to, from, n);
 | 
							res = raw_copy_from_user(to, from, n);
 | 
				
			||||||
		instrument_copy_from_user_after(to, from, n, res);
 | 
							instrument_copy_from_user_after(to, from, n, res);
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue