Re: [PATCH v3 2/4] x86/syscalls: Specific usage of verify_pre_usermode_state

[off-list ref],Ingo Molnar [off-list ref],Paolo Bonzini [off-list ref],Dmitry Safonov [off-list ref],Borislav Petkov [off-list ref],Josh Poimboeuf [off-list ref],Brian Gerst [off-list ref],Jan Beulich [off-list ref],Christian Borntraeger [off-list ref],Fenghua Yu [off-list ref],He Chen [off-list ref],Russell King [off-list ref],Vladimir Murzin [off-list ref],Will Deacon [off-list ref],Catalin Marinas [off-list ref],Mark Rutland [off-list ref],James Morse [off-list ref],"David A . Long" [off-list ref],Pratyush Anand [off-list ref],Laura Abbott [off-list ref],Andre Przywara [off-list ref],Chris Metcalf [off-list ref],linux-
 s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8@public.gmane.org
From: hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org
Message-ID: [off-list ref]

On March 11, 2017 1:42:00 AM PST, Ingo Molnar [off-list ref] wrote:

* Thomas Garnier [off-list ref] wrote:

quoted

Implement specific usage of verify_pre_usermode_state for user-mode
returns for x86.
---
Based on next-20170308
---
 arch/x86/Kconfig                        |  1 +
 arch/x86/entry/common.c                 |  3 +++
 arch/x86/entry/entry_64.S               | 19 +++++++++++++++++++
 arch/x86/include/asm/pgtable_64_types.h | 11 +++++++++++
 arch/x86/include/asm/processor.h        | 11 -----------
 5 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 005df7c825f5..6d48e18e6f09 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig

@@ -63,6 +63,7 @@ config X86
 	select ARCH_MIGHT_HAVE_ACPI_PDC		if ACPI
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_MIGHT_HAVE_PC_SERIO
+	select ARCH_NO_SYSCALL_VERIFY_PRE_USERMODE_STATE
 	select ARCH_SUPPORTS_ATOMIC_RMW
 	select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
 	select ARCH_SUPPORTS_NUMA_BALANCING	if X86_64

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 370c42c7f046..525edbb77f03 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c

@@ -22,6 +22,7 @@
 #include <linux/context_tracking.h>
 #include <linux/user-return-notifier.h>
 #include <linux/uprobes.h>
+#include <linux/syscalls.h>
 
 #include <asm/desc.h>
 #include <asm/traps.h>

@@ -180,6 +181,8 @@ __visible inline void

prepare_exit_to_usermode(struct pt_regs *regs)

quoted

 	struct thread_info *ti = current_thread_info();
 	u32 cached_flags;
 
+	verify_pre_usermode_state();
+
 	if (IS_ENABLED(CONFIG_PROVE_LOCKING) && WARN_ON(!irqs_disabled()))
 		local_irq_disable();
 

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index d2b2a2948ffe..04db589be466 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S

@@ -218,6 +218,25 @@ entry_SYSCALL_64_fastpath:
 	testl	$_TIF_ALLWORK_MASK, TASK_TI_flags(%r11)
 	jnz	1f
 
+	/*
+	 * Check user-mode state on fast path return, the same check is

done

quoted

+	 * under the slow path through syscall_return_slowpath.
+	 */
+#ifdef CONFIG_BUG_ON_DATA_CORRUPTION
+	call	verify_pre_usermode_state
+#else
+	/*
+	 * Similar to set_fs(USER_DS) in verify_pre_usermode_state without

a

quoted

+	 * warning.
+	 */
+	movq	PER_CPU_VAR(current_task), %rax
+	movq	$TASK_SIZE_MAX, %rcx
+	cmp	%rcx, TASK_addr_limit(%rax)
+	jz	1f
+	movq	%rcx, TASK_addr_limit(%rax)
+1:
+#endif
+
 	LOCKDEP_SYS_EXIT
 	TRACE_IRQS_ON		/* user mode is traced as IRQs on */
 	movq	RIP(%rsp), %rcx

Ugh, so you call an assembly function just to ... call another
function.

Plus why is it in assembly to begin with? Is this some older code that
got
written when the x86 entry code was in assembly, and never properly
converted to C?

Thanks,

Ingo

The code does a compare to jump around a store.  It would be much cleaner and faster to simply clobber the value unconditionally.  If there is a test it should be to avoid the function call, not (only) the assignment.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.