Commit 6c750ed0 authored Feb 21, 2023 by Dave Hansen Committed by Greg Kroah-Hartman Feb 25, 2023

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>

parent 4d2e5de0

include/linux/nospec.h

+4 −0

Original line number	Original line	Diff line number	Diff line
	@@ -9,6 +9,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

kernel/bpf/core.c

+0 −2

Original line number	Original line	Diff line number	Diff line
	@@ -1567,9 +1567,7 @@ static u64 ___bpf_prog_run(u64 regs, const struct bpf_insn insn, u64 *stack)
	* 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: \

lib/usercopy.c

+7 −0

Original line number	Original line	Diff line number	Diff line
	// SPDX-License-Identifier: GPL-2.0		// SPDX-License-Identifier: GPL-2.0
	#include <linux/uaccess.h>		#include <linux/uaccess.h>
	#include <linux/bitops.h>		#include <linux/bitops.h>
			#include <linux/nospec.h>

	/* out-of-line parts */		/* 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;		unsigned long res = n;
	might_fault();		might_fault();
	if (likely(access_ok(from, n))) {		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);		kasan_check_write(to, n);
	res = raw_copy_from_user(to, from, n);		res = raw_copy_from_user(to, from, n);
	}		}