From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev, Jordy Zomer <jordyzomer@google.com>,
	Linus Torvalds <torvalds@linuxfoundation.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Daniel Borkmann <daniel@iogearbox.net>
Subject: [PATCH 5.4 15/18] uaccess: Add speculation barrier to copy_from_user()
Date: Thu, 23 Feb 2023 14:07:00 +0100	[thread overview]
Message-ID: <20230223130426.256164730@linuxfoundation.org> (raw)
In-Reply-To: <20230223130425.680784802@linuxfoundation.org>
From: Dave Hansen <dave.hansen@linux.intel.com>
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>
---
 include/linux/nospec.h |    4 ++++
 kernel/bpf/core.c      |    2 --
 lib/usercopy.c         |    7 +++++++
 3 files changed, 11 insertions(+), 2 deletions(-)
--- 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
--- 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:						\
--- 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,
 	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);
 	}
next prev parent reply	other threads:[~2023-02-23 13:11 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-23 13:06 [PATCH 5.4 00/18] 5.4.233-rc1 review Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 01/18] dma-mapping: add generic helpers for mapping sgtable objects Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 02/18] scatterlist: add generic wrappers for iterating over " Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 03/18] drm: etnaviv: fix common struct sg_table related issues Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 04/18] drm/etnaviv: dont truncate physical page address Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 05/18] wifi: rtl8xxxu: gen2: Turn on the rate control Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 06/18] powerpc: dts: t208x: Mark MAC1 and MAC2 as 10G Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 07/18] random: always mix cycle counter in add_latent_entropy() Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 08/18] KVM: x86: Fail emulation during EMULTYPE_SKIP on any exception Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 09/18] KVM: VMX: Execute IBPB on emulated VM-exit when guest has IBRS Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 10/18] can: kvaser_usb: hydra: help gcc-13 to figure out cmd_len Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 11/18] powerpc: dts: t208x: Disable 10G on MAC1 and MAC2 Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 12/18] alarmtimer: Prevent starvation by small intervals and SIG_IGN Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 13/18] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry Greg Kroah-Hartman
2023-02-23 13:06 ` [PATCH 5.4 14/18] mac80211: mesh: embedd mesh_paths and mpp_paths into ieee80211_if_mesh Greg Kroah-Hartman
2023-02-23 13:07 ` Greg Kroah-Hartman [this message]
2023-02-23 13:07 ` [PATCH 5.4 16/18] wifi: mwifiex: Add missing compatible string for SD8787 Greg Kroah-Hartman
2023-02-23 13:07 ` [PATCH 5.4 17/18] ext4: Fix function prototype mismatch for ext4_feat_ktype Greg Kroah-Hartman
2023-02-23 13:07 ` [PATCH 5.4 18/18] Revert "net/sched: taprio: make qdisc_leaf() see the per-netdev-queue pfifo child qdiscs" Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox
  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):
  git send-email \
    --in-reply-to=20230223130426.256164730@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=daniel@iogearbox.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=jordyzomer@google.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=torvalds@linuxfoundation.org \
    /path/to/YOUR_REPLY
  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
  Be sure your reply has a Subject: header at the top and a blank line
  before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).