All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/9] ARM: spectre-v1.1 mitigations
@ 2018-09-10 10:42 Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 1/9] ARM: signal: copy registers using __copy_to_user() Julien Thierry
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

The series provides mitigations for spectre-v1.1. It is mostly mirroring
what Russell did [1], but this time for writes to user provided addresses.

* Patches 1-5 replace some __put_user_error calls with __copy_to_user, this
  both reduces the number of time address sanitizing is performed and also
  the number of time PAN needs to be toggled.

* Patches 6-9 ensure user addresses used by __put_user* and
  __copy_to_user functions are sanitized before being used.

Changes since v2[2]:
- Ensure padding in vfp_sigframe is properly initialized
- Fix incorrect copy of oabi events

Changes since v1[3]:
- Fixed off-by-one error in uaccess_mask_range_ptr, spotted by Russell
- Remove remaining calls to __put_user_error() and get rid of the macro
- Reorder the patches to better reflect the spectre-v1 series

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-July/589869.html
[2] https://www.spinics.net/lists/arm-kernel/msg674995.html
[3] https://www.spinics.net/lists/arm-kernel/msg671026.html

Cheers,

Julien


Julien Thierry (9):
  ARM: signal: copy registers using __copy_to_user()
  ARM: signal: always use __copy_to_user to save iwmmxt context
  ARM: vfp: use __copy_to_user() when saving VFP state
  ARM: oabi-compat: copy oabi events using __copy_to_user()
  ARM: signal: replace __put_user_error with __put_user
  ARM: uaccess: Prevent speculative use of the current addr_limit
  ARM: spectre-v1.1: use put_user() for __put_user()
  ARM: spectre-v1,v1.1: provide helpers for address sanitization
  ARM: spectre-v1.1: harden __copy_to_user

 arch/arm/include/asm/assembler.h   | 11 ++++++
 arch/arm/include/asm/thread_info.h |  4 +-
 arch/arm/include/asm/uaccess.h     | 49 ++++++++++++++++++++---
 arch/arm/kernel/signal.c           | 80 +++++++++++++++++++++-----------------
 arch/arm/kernel/sys_oabi-compat.c  |  8 +++-
 arch/arm/lib/copy_from_user.S      |  6 +--
 arch/arm/lib/copy_to_user.S        |  6 ++-
 arch/arm/lib/uaccess_with_memcpy.c |  3 +-
 arch/arm/vfp/vfpmodule.c           | 20 ++++------
 9 files changed, 122 insertions(+), 65 deletions(-)

--
1.9.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/9] ARM: signal: copy registers using __copy_to_user()
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
@ 2018-09-10 10:42 ` Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 2/9] ARM: signal: always use __copy_to_user to save iwmmxt context Julien Thierry
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

When saving the ARM integer registers, use __copy_to_user() to
copy them into user signal frame, rather than __put_user_error().
This has the benefit of disabling/enabling PAN once for the whole copy
intead of once per write.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arch/arm/kernel/signal.c | 49 ++++++++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index b8f766c..76fe75d 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -288,30 +288,35 @@ asmlinkage int sys_rt_sigreturn(struct pt_regs *regs)
 setup_sigframe(struct sigframe __user *sf, struct pt_regs *regs, sigset_t *set)
 {
 	struct aux_sigframe __user *aux;
+	struct sigcontext context;
 	int err = 0;
 
-	__put_user_error(regs->ARM_r0, &sf->uc.uc_mcontext.arm_r0, err);
-	__put_user_error(regs->ARM_r1, &sf->uc.uc_mcontext.arm_r1, err);
-	__put_user_error(regs->ARM_r2, &sf->uc.uc_mcontext.arm_r2, err);
-	__put_user_error(regs->ARM_r3, &sf->uc.uc_mcontext.arm_r3, err);
-	__put_user_error(regs->ARM_r4, &sf->uc.uc_mcontext.arm_r4, err);
-	__put_user_error(regs->ARM_r5, &sf->uc.uc_mcontext.arm_r5, err);
-	__put_user_error(regs->ARM_r6, &sf->uc.uc_mcontext.arm_r6, err);
-	__put_user_error(regs->ARM_r7, &sf->uc.uc_mcontext.arm_r7, err);
-	__put_user_error(regs->ARM_r8, &sf->uc.uc_mcontext.arm_r8, err);
-	__put_user_error(regs->ARM_r9, &sf->uc.uc_mcontext.arm_r9, err);
-	__put_user_error(regs->ARM_r10, &sf->uc.uc_mcontext.arm_r10, err);
-	__put_user_error(regs->ARM_fp, &sf->uc.uc_mcontext.arm_fp, err);
-	__put_user_error(regs->ARM_ip, &sf->uc.uc_mcontext.arm_ip, err);
-	__put_user_error(regs->ARM_sp, &sf->uc.uc_mcontext.arm_sp, err);
-	__put_user_error(regs->ARM_lr, &sf->uc.uc_mcontext.arm_lr, err);
-	__put_user_error(regs->ARM_pc, &sf->uc.uc_mcontext.arm_pc, err);
-	__put_user_error(regs->ARM_cpsr, &sf->uc.uc_mcontext.arm_cpsr, err);
-
-	__put_user_error(current->thread.trap_no, &sf->uc.uc_mcontext.trap_no, err);
-	__put_user_error(current->thread.error_code, &sf->uc.uc_mcontext.error_code, err);
-	__put_user_error(current->thread.address, &sf->uc.uc_mcontext.fault_address, err);
-	__put_user_error(set->sig[0], &sf->uc.uc_mcontext.oldmask, err);
+	context = (struct sigcontext) {
+		.arm_r0        = regs->ARM_r0,
+		.arm_r1        = regs->ARM_r1,
+		.arm_r2        = regs->ARM_r2,
+		.arm_r3        = regs->ARM_r3,
+		.arm_r4        = regs->ARM_r4,
+		.arm_r5        = regs->ARM_r5,
+		.arm_r6        = regs->ARM_r6,
+		.arm_r7        = regs->ARM_r7,
+		.arm_r8        = regs->ARM_r8,
+		.arm_r9        = regs->ARM_r9,
+		.arm_r10       = regs->ARM_r10,
+		.arm_fp        = regs->ARM_fp,
+		.arm_ip        = regs->ARM_ip,
+		.arm_sp        = regs->ARM_sp,
+		.arm_lr        = regs->ARM_lr,
+		.arm_pc        = regs->ARM_pc,
+		.arm_cpsr      = regs->ARM_cpsr,
+
+		.trap_no       = current->thread.trap_no,
+		.error_code    = current->thread.error_code,
+		.fault_address = current->thread.address,
+		.oldmask       = set->sig[0],
+	};
+
+	err |= __copy_to_user(&sf->uc.uc_mcontext, &context, sizeof(context));
 
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/9] ARM: signal: always use __copy_to_user to save iwmmxt context
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 1/9] ARM: signal: copy registers using __copy_to_user() Julien Thierry
@ 2018-09-10 10:42 ` Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 3/9] ARM: vfp: use __copy_to_user() when saving VFP state Julien Thierry
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

When setting a dummy iwmmxt context, create a local instance and
use __copy_to_user both cases whether iwmmxt is being used or not.
This has the benefit of disabling/enabling PAN once for the whole copy
intead of once per write.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arch/arm/kernel/signal.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index 76fe75d..464393d 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -77,8 +77,6 @@ static int preserve_iwmmxt_context(struct iwmmxt_sigframe __user *frame)
 		kframe->magic = IWMMXT_MAGIC;
 		kframe->size = IWMMXT_STORAGE_SIZE;
 		iwmmxt_task_copy(current_thread_info(), &kframe->storage);
-
-		err = __copy_to_user(frame, kframe, sizeof(*frame));
 	} else {
 		/*
 		 * For bug-compatibility with older kernels, some space
@@ -86,10 +84,14 @@ static int preserve_iwmmxt_context(struct iwmmxt_sigframe __user *frame)
 		 * Set the magic and size appropriately so that properly
 		 * written userspace can skip it reliably:
 		 */
-		__put_user_error(DUMMY_MAGIC, &frame->magic, err);
-		__put_user_error(IWMMXT_STORAGE_SIZE, &frame->size, err);
+		*kframe = (struct iwmmxt_sigframe) {
+			.magic = DUMMY_MAGIC,
+			.size  = IWMMXT_STORAGE_SIZE,
+		};
 	}
 
+	err = __copy_to_user(frame, kframe, sizeof(*kframe));
+
 	return err;
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/9] ARM: vfp: use __copy_to_user() when saving VFP state
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 1/9] ARM: signal: copy registers using __copy_to_user() Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 2/9] ARM: signal: always use __copy_to_user to save iwmmxt context Julien Thierry
@ 2018-09-10 10:42 ` Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 4/9] ARM: oabi-compat: copy oabi events using __copy_to_user() Julien Thierry
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

Use __copy_to_user() rather than __put_user_error() for individual
members when saving VFP state.
This has the benefit of disabling/enabling PAN once per copied struct
intead of once per write.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arch/arm/include/asm/thread_info.h |  4 ++--
 arch/arm/kernel/signal.c           | 13 +++++++------
 arch/arm/vfp/vfpmodule.c           | 20 ++++++++------------
 3 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index 9b37b6a..8f55dc5 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -121,8 +121,8 @@ static inline struct thread_info *current_thread_info(void)
 struct user_vfp;
 struct user_vfp_exc;
 
-extern int vfp_preserve_user_clear_hwstate(struct user_vfp __user *,
-					   struct user_vfp_exc __user *);
+extern int vfp_preserve_user_clear_hwstate(struct user_vfp *,
+					   struct user_vfp_exc *);
 extern int vfp_restore_user_hwstate(struct user_vfp *,
 				    struct user_vfp_exc *);
 #endif
diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index 464393d..1e2ecfe 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -137,17 +137,18 @@ static int restore_iwmmxt_context(char __user **auxp)
 
 static int preserve_vfp_context(struct vfp_sigframe __user *frame)
 {
-	const unsigned long magic = VFP_MAGIC;
-	const unsigned long size = VFP_STORAGE_SIZE;
+	struct vfp_sigframe kframe;
 	int err = 0;
 
-	__put_user_error(magic, &frame->magic, err);
-	__put_user_error(size, &frame->size, err);
+	memset(&kframe, 0, sizeof(kframe));
+	kframe.magic = VFP_MAGIC;
+	kframe.size = VFP_STORAGE_SIZE;
 
+	err = vfp_preserve_user_clear_hwstate(&kframe.ufp, &kframe.ufp_exc);
 	if (err)
-		return -EFAULT;
+		return err;
 
-	return vfp_preserve_user_clear_hwstate(&frame->ufp, &frame->ufp_exc);
+	return __copy_to_user(frame, &kframe, sizeof(kframe));
 }
 
 static int restore_vfp_context(char __user **auxp)
diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index dc7e6b5..2b287d0 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -553,12 +553,11 @@ void vfp_flush_hwstate(struct thread_info *thread)
  * Save the current VFP state into the provided structures and prepare
  * for entry into a new function (signal handler).
  */
-int vfp_preserve_user_clear_hwstate(struct user_vfp __user *ufp,
-				    struct user_vfp_exc __user *ufp_exc)
+int vfp_preserve_user_clear_hwstate(struct user_vfp *ufp,
+				    struct user_vfp_exc *ufp_exc)
 {
 	struct thread_info *thread = current_thread_info();
 	struct vfp_hard_struct *hwstate = &thread->vfpstate.hard;
-	int err = 0;
 
 	/* Ensure that the saved hwstate is up-to-date. */
 	vfp_sync_hwstate(thread);
@@ -567,22 +566,19 @@ int vfp_preserve_user_clear_hwstate(struct user_vfp __user *ufp,
 	 * Copy the floating point registers. There can be unused
 	 * registers see asm/hwcap.h for details.
 	 */
-	err |= __copy_to_user(&ufp->fpregs, &hwstate->fpregs,
-			      sizeof(hwstate->fpregs));
+	memcpy(&ufp->fpregs, &hwstate->fpregs, sizeof(hwstate->fpregs));
+
 	/*
 	 * Copy the status and control register.
 	 */
-	__put_user_error(hwstate->fpscr, &ufp->fpscr, err);
+	ufp->fpscr = hwstate->fpscr;
 
 	/*
 	 * Copy the exception registers.
 	 */
-	__put_user_error(hwstate->fpexc, &ufp_exc->fpexc, err);
-	__put_user_error(hwstate->fpinst, &ufp_exc->fpinst, err);
-	__put_user_error(hwstate->fpinst2, &ufp_exc->fpinst2, err);
-
-	if (err)
-		return -EFAULT;
+	ufp_exc->fpexc = hwstate->fpexc;
+	ufp_exc->fpinst = hwstate->fpinst;
+	ufp_exc->fpinst2 = ufp_exc->fpinst2;
 
 	/* Ensure that VFP is disabled. */
 	vfp_flush_hwstate(thread);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/9] ARM: oabi-compat: copy oabi events using __copy_to_user()
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
                   ` (2 preceding siblings ...)
  2018-09-10 10:42 ` [PATCH v3 3/9] ARM: vfp: use __copy_to_user() when saving VFP state Julien Thierry
@ 2018-09-10 10:42 ` Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 5/9] ARM: signal: replace __put_user_error with __put_user Julien Thierry
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

Copy events to user using __copy_to_user() rather than copy members of
individually with __put_user_error().
This has the benefit of disabling/enabling PAN once per event intead of
once per event member.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arch/arm/kernel/sys_oabi-compat.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/sys_oabi-compat.c b/arch/arm/kernel/sys_oabi-compat.c
index f0dd4b6..40da087 100644
--- a/arch/arm/kernel/sys_oabi-compat.c
+++ b/arch/arm/kernel/sys_oabi-compat.c
@@ -277,6 +277,7 @@ asmlinkage long sys_oabi_epoll_wait(int epfd,
 				    int maxevents, int timeout)
 {
 	struct epoll_event *kbuf;
+	struct oabi_epoll_event e;
 	mm_segment_t fs;
 	long ret, err, i;
 
@@ -295,8 +296,11 @@ asmlinkage long sys_oabi_epoll_wait(int epfd,
 	set_fs(fs);
 	err = 0;
 	for (i = 0; i < ret; i++) {
-		__put_user_error(kbuf[i].events, &events->events, err);
-		__put_user_error(kbuf[i].data,   &events->data,   err);
+		e.events = kbuf[i].events;
+		e.data = kbuf[i].data;
+		err = __copy_to_user(events, &e, sizeof(e));
+		if (err)
+			break;
 		events++;
 	}
 	kfree(kbuf);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 5/9] ARM: signal: replace __put_user_error with __put_user
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
                   ` (3 preceding siblings ...)
  2018-09-10 10:42 ` [PATCH v3 4/9] ARM: oabi-compat: copy oabi events using __copy_to_user() Julien Thierry
@ 2018-09-10 10:42 ` Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 6/9] ARM: uaccess: Prevent speculative use of the current addr_limit Julien Thierry
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

With Spectre-v1.1 mitigations, __put_user_error is pointless. In an attempt
to remove it, replace its references in frame setups with __put_user.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arch/arm/kernel/signal.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index 1e2ecfe..b908382 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -336,7 +336,7 @@ asmlinkage int sys_rt_sigreturn(struct pt_regs *regs)
 	if (err == 0)
 		err |= preserve_vfp_context(&aux->vfp);
 #endif
-	__put_user_error(0, &aux->end_magic, err);
+	err |= __put_user(0, &aux->end_magic);
 
 	return err;
 }
@@ -499,7 +499,7 @@ asmlinkage int sys_rt_sigreturn(struct pt_regs *regs)
 	/*
 	 * Set uc.uc_flags to a value which sc.trap_no would never have.
 	 */
-	__put_user_error(0x5ac3c35a, &frame->uc.uc_flags, err);
+	err = __put_user(0x5ac3c35a, &frame->uc.uc_flags);
 
 	err |= setup_sigframe(frame, regs, set);
 	if (err == 0)
@@ -519,8 +519,8 @@ asmlinkage int sys_rt_sigreturn(struct pt_regs *regs)
 
 	err |= copy_siginfo_to_user(&frame->info, &ksig->info);
 
-	__put_user_error(0, &frame->sig.uc.uc_flags, err);
-	__put_user_error(NULL, &frame->sig.uc.uc_link, err);
+	err |= __put_user(0, &frame->sig.uc.uc_flags);
+	err |= __put_user(NULL, &frame->sig.uc.uc_link);
 
 	err |= __save_altstack(&frame->sig.uc.uc_stack, regs->ARM_sp);
 	err |= setup_sigframe(&frame->sig, regs, set);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 6/9] ARM: uaccess: Prevent speculative use of the current addr_limit
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
                   ` (4 preceding siblings ...)
  2018-09-10 10:42 ` [PATCH v3 5/9] ARM: signal: replace __put_user_error with __put_user Julien Thierry
@ 2018-09-10 10:42 ` Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 7/9] ARM: spectre-v1.1: use put_user() for __put_user() Julien Thierry
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

A mispredicted conditional call to set_fs could result in the wrong
addr_limit being forwarded under speculation to a subsequent access_ok
check, potentially forming part of a spectre-v1 attack using uaccess
routines.

This patch prevents this forwarding from taking place, but putting heavy
barriers in set_fs after writing the addr_limit.

Porting commit c2f0ad4fc089cff8 ("arm64: uaccess: Prevent speculative use
of the current addr_limit").

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arch/arm/include/asm/uaccess.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 5451e1f..d65ef85 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -69,6 +69,14 @@ static inline void uaccess_restore(unsigned int flags)
 static inline void set_fs(mm_segment_t fs)
 {
 	current_thread_info()->addr_limit = fs;
+
+	/*
+	 * Prevent a mispredicted conditional call to set_fs from forwarding
+	 * the wrong address limit to access_ok under speculation.
+	 */
+	dsb(nsh);
+	isb();
+
 	modify_domain(DOMAIN_KERNEL, fs ? DOMAIN_CLIENT : DOMAIN_MANAGER);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 7/9] ARM: spectre-v1.1: use put_user() for __put_user()
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
                   ` (5 preceding siblings ...)
  2018-09-10 10:42 ` [PATCH v3 6/9] ARM: uaccess: Prevent speculative use of the current addr_limit Julien Thierry
@ 2018-09-10 10:42 ` Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 8/9] ARM: spectre-v1, v1.1: provide helpers for address sanitization Julien Thierry
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

When Spectre mitigation is required, __put_user() needs to include
check_uaccess. This is already the case for put_user(), so just make
__put_user() an alias of put_user().

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arch/arm/include/asm/uaccess.h | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index d65ef85..1191e7d 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -370,6 +370,14 @@ static inline void set_fs(mm_segment_t fs)
 	__pu_err;							\
 })
 
+#ifdef CONFIG_CPU_SPECTRE
+/*
+ * When mitigating Spectre variant 1.1, all accessors need to include
+ * verification of the address space.
+ */
+#define __put_user(x, ptr) put_user(x, ptr)
+
+#else
 #define __put_user(x, ptr)						\
 ({									\
 	long __pu_err = 0;						\
@@ -377,12 +385,6 @@ static inline void set_fs(mm_segment_t fs)
 	__pu_err;							\
 })
 
-#define __put_user_error(x, ptr, err)					\
-({									\
-	__put_user_switch((x), (ptr), (err), __put_user_nocheck);	\
-	(void) 0;							\
-})
-
 #define __put_user_nocheck(x, __pu_ptr, __err, __size)			\
 	do {								\
 		unsigned long __pu_addr = (unsigned long)__pu_ptr;	\
@@ -462,6 +464,7 @@ static inline void set_fs(mm_segment_t fs)
 	: "r" (x), "i" (-EFAULT)				\
 	: "cc")
 
+#endif /* !CONFIG_CPU_SPECTRE */
 
 #ifdef CONFIG_MMU
 extern unsigned long __must_check
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 8/9] ARM: spectre-v1, v1.1: provide helpers for address sanitization
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
                   ` (6 preceding siblings ...)
  2018-09-10 10:42 ` [PATCH v3 7/9] ARM: spectre-v1.1: use put_user() for __put_user() Julien Thierry
@ 2018-09-10 10:42 ` Julien Thierry
  2018-09-10 10:42 ` [PATCH v3 9/9] ARM: spectre-v1.1: harden __copy_to_user Julien Thierry
  2018-09-10 16:35 ` [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Russell King - ARM Linux
  9 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

Introduce C and asm helpers to sanitize user address, taking the
address range they target into account.

Use asm helper for existing sanitization in __copy_from_user().

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arch/arm/include/asm/assembler.h | 11 +++++++++++
 arch/arm/include/asm/uaccess.h   | 26 ++++++++++++++++++++++++++
 arch/arm/lib/copy_from_user.S    |  6 +-----
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index b17ee03..88286dd 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -467,6 +467,17 @@
 #endif
 	.endm
 
+	.macro uaccess_mask_range_ptr, addr:req, size:req, limit:req, tmp:req
+#ifdef CONFIG_CPU_SPECTRE
+	sub	\tmp, \limit, #1
+	subs	\tmp, \tmp, \addr	@ tmp = limit - 1 - addr
+	addhs	\tmp, \tmp, #1		@ if (tmp >= 0) {
+	subhss	\tmp, \tmp, \size	@ tmp = limit - (addr + size) }
+	movlo	\addr, #0		@ if (tmp < 0) addr = NULL
+	csdb
+#endif
+	.endm
+
 	.macro	uaccess_disable, tmp, isb=1
 #ifdef CONFIG_CPU_SW_DOMAIN_PAN
 	/*
diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 1191e7d..c136eef 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -100,6 +100,32 @@ static inline void set_fs(mm_segment_t fs)
 	__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
 
 /*
+ * Sanitise a uaccess pointer such that it becomes NULL if addr+size
+ * is above the current addr_limit.
+ */
+#define uaccess_mask_range_ptr(ptr, size)			\
+	((__typeof__(ptr))__uaccess_mask_range_ptr(ptr, size))
+static inline void __user *__uaccess_mask_range_ptr(const void __user *ptr,
+						    size_t size)
+{
+	void __user *safe_ptr = (void __user *)ptr;
+	unsigned long tmp;
+
+	asm volatile(
+	"	sub	%1, %3, #1\n"
+	"	subs	%1, %1, %0\n"
+	"	addhs	%1, %1, #1\n"
+	"	subhss	%1, %1, %2\n"
+	"	movlo	%0, #0\n"
+	: "+r" (safe_ptr), "=&r" (tmp)
+	: "r" (size), "r" (current_thread_info()->addr_limit)
+	: "cc");
+
+	csdb();
+	return safe_ptr;
+}
+
+/*
  * Single-value transfer routines.  They automatically use the right
  * size if we just have the right pointer type.  Note that the functions
  * which read from user space (*get_*) need to take care not to leak
diff --git a/arch/arm/lib/copy_from_user.S b/arch/arm/lib/copy_from_user.S
index a826df3..6709a8d 100644
--- a/arch/arm/lib/copy_from_user.S
+++ b/arch/arm/lib/copy_from_user.S
@@ -93,11 +93,7 @@ ENTRY(arm_copy_from_user)
 #ifdef CONFIG_CPU_SPECTRE
 	get_thread_info r3
 	ldr	r3, [r3, #TI_ADDR_LIMIT]
-	adds	ip, r1, r2	@ ip=addr+size
-	sub	r3, r3, #1	@ addr_limit - 1
-	cmpcc	ip, r3		@ if (addr+size > addr_limit - 1)
-	movcs	r1, #0		@ addr = NULL
-	csdb
+	uaccess_mask_range_ptr r1, r2, r3, ip
 #endif
 
 #include "copy_template.S"
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 9/9] ARM: spectre-v1.1: harden __copy_to_user
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
                   ` (7 preceding siblings ...)
  2018-09-10 10:42 ` [PATCH v3 8/9] ARM: spectre-v1, v1.1: provide helpers for address sanitization Julien Thierry
@ 2018-09-10 10:42 ` Julien Thierry
  2018-09-10 16:35 ` [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Russell King - ARM Linux
  9 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-10 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

Sanitize user pointer given to __copy_to_user, both for standard version
and memcopy version of the user accessor.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arch/arm/lib/copy_to_user.S        | 6 +++++-
 arch/arm/lib/uaccess_with_memcpy.c | 3 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/lib/copy_to_user.S b/arch/arm/lib/copy_to_user.S
index caf5019..970abe5 100644
--- a/arch/arm/lib/copy_to_user.S
+++ b/arch/arm/lib/copy_to_user.S
@@ -94,6 +94,11 @@
 
 ENTRY(__copy_to_user_std)
 WEAK(arm_copy_to_user)
+#ifdef CONFIG_CPU_SPECTRE
+	get_thread_info r3
+	ldr	r3, [r3, #TI_ADDR_LIMIT]
+	uaccess_mask_range_ptr r0, r2, r3, ip
+#endif
 
 #include "copy_template.S"
 
@@ -108,4 +113,3 @@ ENDPROC(__copy_to_user_std)
 	rsb	r0, r0, r2
 	copy_abort_end
 	.popsection
-
diff --git a/arch/arm/lib/uaccess_with_memcpy.c b/arch/arm/lib/uaccess_with_memcpy.c
index 9b4ed17..73dc736 100644
--- a/arch/arm/lib/uaccess_with_memcpy.c
+++ b/arch/arm/lib/uaccess_with_memcpy.c
@@ -152,7 +152,8 @@
 		n = __copy_to_user_std(to, from, n);
 		uaccess_restore(ua_flags);
 	} else {
-		n = __copy_to_user_memcpy(to, from, n);
+		n = __copy_to_user_memcpy(uaccess_mask_range_ptr(to, n),
+					  from, n);
 	}
 	return n;
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 0/9] ARM: spectre-v1.1 mitigations
  2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
                   ` (8 preceding siblings ...)
  2018-09-10 10:42 ` [PATCH v3 9/9] ARM: spectre-v1.1: harden __copy_to_user Julien Thierry
@ 2018-09-10 16:35 ` Russell King - ARM Linux
  2018-09-11  9:18   ` Julien Thierry
  9 siblings, 1 reply; 12+ messages in thread
From: Russell King - ARM Linux @ 2018-09-10 16:35 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Julien,

I think this now looks fine, please drop it into the patch system.
Thanks.

On Mon, Sep 10, 2018 at 11:42:39AM +0100, Julien Thierry wrote:
> Hi,
> 
> The series provides mitigations for spectre-v1.1. It is mostly mirroring
> what Russell did [1], but this time for writes to user provided addresses.
> 
> * Patches 1-5 replace some __put_user_error calls with __copy_to_user, this
>   both reduces the number of time address sanitizing is performed and also
>   the number of time PAN needs to be toggled.
> 
> * Patches 6-9 ensure user addresses used by __put_user* and
>   __copy_to_user functions are sanitized before being used.
> 
> Changes since v2[2]:
> - Ensure padding in vfp_sigframe is properly initialized
> - Fix incorrect copy of oabi events
> 
> Changes since v1[3]:
> - Fixed off-by-one error in uaccess_mask_range_ptr, spotted by Russell
> - Remove remaining calls to __put_user_error() and get rid of the macro
> - Reorder the patches to better reflect the spectre-v1 series
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-July/589869.html
> [2] https://www.spinics.net/lists/arm-kernel/msg674995.html
> [3] https://www.spinics.net/lists/arm-kernel/msg671026.html
> 
> Cheers,
> 
> Julien
> 
> 
> Julien Thierry (9):
>   ARM: signal: copy registers using __copy_to_user()
>   ARM: signal: always use __copy_to_user to save iwmmxt context
>   ARM: vfp: use __copy_to_user() when saving VFP state
>   ARM: oabi-compat: copy oabi events using __copy_to_user()
>   ARM: signal: replace __put_user_error with __put_user
>   ARM: uaccess: Prevent speculative use of the current addr_limit
>   ARM: spectre-v1.1: use put_user() for __put_user()
>   ARM: spectre-v1,v1.1: provide helpers for address sanitization
>   ARM: spectre-v1.1: harden __copy_to_user
> 
>  arch/arm/include/asm/assembler.h   | 11 ++++++
>  arch/arm/include/asm/thread_info.h |  4 +-
>  arch/arm/include/asm/uaccess.h     | 49 ++++++++++++++++++++---
>  arch/arm/kernel/signal.c           | 80 +++++++++++++++++++++-----------------
>  arch/arm/kernel/sys_oabi-compat.c  |  8 +++-
>  arch/arm/lib/copy_from_user.S      |  6 +--
>  arch/arm/lib/copy_to_user.S        |  6 ++-
>  arch/arm/lib/uaccess_with_memcpy.c |  3 +-
>  arch/arm/vfp/vfpmodule.c           | 20 ++++------
>  9 files changed, 122 insertions(+), 65 deletions(-)
> 
> --
> 1.9.1

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 0/9] ARM: spectre-v1.1 mitigations
  2018-09-10 16:35 ` [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Russell King - ARM Linux
@ 2018-09-11  9:18   ` Julien Thierry
  0 siblings, 0 replies; 12+ messages in thread
From: Julien Thierry @ 2018-09-11  9:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,

On 10/09/18 17:35, Russell King - ARM Linux wrote:
> Hi Julien,
> 
> I think this now looks fine, please drop it into the patch system.
> Thanks.
> 

The patches are in the patch system now. Thanks for your reviews.

Thanks,

> On Mon, Sep 10, 2018 at 11:42:39AM +0100, Julien Thierry wrote:
>> Hi,
>>
>> The series provides mitigations for spectre-v1.1. It is mostly mirroring
>> what Russell did [1], but this time for writes to user provided addresses.
>>
>> * Patches 1-5 replace some __put_user_error calls with __copy_to_user, this
>>    both reduces the number of time address sanitizing is performed and also
>>    the number of time PAN needs to be toggled.
>>
>> * Patches 6-9 ensure user addresses used by __put_user* and
>>    __copy_to_user functions are sanitized before being used.
>>
>> Changes since v2[2]:
>> - Ensure padding in vfp_sigframe is properly initialized
>> - Fix incorrect copy of oabi events
>>
>> Changes since v1[3]:
>> - Fixed off-by-one error in uaccess_mask_range_ptr, spotted by Russell
>> - Remove remaining calls to __put_user_error() and get rid of the macro
>> - Reorder the patches to better reflect the spectre-v1 series
>>
>> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-July/589869.html
>> [2] https://www.spinics.net/lists/arm-kernel/msg674995.html
>> [3] https://www.spinics.net/lists/arm-kernel/msg671026.html
>>
>> Cheers,
>>
>> Julien
>>
>>
>> Julien Thierry (9):
>>    ARM: signal: copy registers using __copy_to_user()
>>    ARM: signal: always use __copy_to_user to save iwmmxt context
>>    ARM: vfp: use __copy_to_user() when saving VFP state
>>    ARM: oabi-compat: copy oabi events using __copy_to_user()
>>    ARM: signal: replace __put_user_error with __put_user
>>    ARM: uaccess: Prevent speculative use of the current addr_limit
>>    ARM: spectre-v1.1: use put_user() for __put_user()
>>    ARM: spectre-v1,v1.1: provide helpers for address sanitization
>>    ARM: spectre-v1.1: harden __copy_to_user
>>
>>   arch/arm/include/asm/assembler.h   | 11 ++++++
>>   arch/arm/include/asm/thread_info.h |  4 +-
>>   arch/arm/include/asm/uaccess.h     | 49 ++++++++++++++++++++---
>>   arch/arm/kernel/signal.c           | 80 +++++++++++++++++++++-----------------
>>   arch/arm/kernel/sys_oabi-compat.c  |  8 +++-
>>   arch/arm/lib/copy_from_user.S      |  6 +--
>>   arch/arm/lib/copy_to_user.S        |  6 ++-
>>   arch/arm/lib/uaccess_with_memcpy.c |  3 +-
>>   arch/arm/vfp/vfpmodule.c           | 20 ++++------
>>   9 files changed, 122 insertions(+), 65 deletions(-)
>>
>> --
>> 1.9.1
> 

-- 
Julien Thierry

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-09-11  9:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-10 10:42 [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Julien Thierry
2018-09-10 10:42 ` [PATCH v3 1/9] ARM: signal: copy registers using __copy_to_user() Julien Thierry
2018-09-10 10:42 ` [PATCH v3 2/9] ARM: signal: always use __copy_to_user to save iwmmxt context Julien Thierry
2018-09-10 10:42 ` [PATCH v3 3/9] ARM: vfp: use __copy_to_user() when saving VFP state Julien Thierry
2018-09-10 10:42 ` [PATCH v3 4/9] ARM: oabi-compat: copy oabi events using __copy_to_user() Julien Thierry
2018-09-10 10:42 ` [PATCH v3 5/9] ARM: signal: replace __put_user_error with __put_user Julien Thierry
2018-09-10 10:42 ` [PATCH v3 6/9] ARM: uaccess: Prevent speculative use of the current addr_limit Julien Thierry
2018-09-10 10:42 ` [PATCH v3 7/9] ARM: spectre-v1.1: use put_user() for __put_user() Julien Thierry
2018-09-10 10:42 ` [PATCH v3 8/9] ARM: spectre-v1, v1.1: provide helpers for address sanitization Julien Thierry
2018-09-10 10:42 ` [PATCH v3 9/9] ARM: spectre-v1.1: harden __copy_to_user Julien Thierry
2018-09-10 16:35 ` [PATCH v3 0/9] ARM: spectre-v1.1 mitigations Russell King - ARM Linux
2018-09-11  9:18   ` Julien Thierry

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.