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

On 03/13/17 17:04, H. Peter Anvin wrote:

On 03/11/17 01:42, Ingo Molnar wrote:

quoted

quoted

 
+	/*
+	 * Check user-mode state on fast path return, the same check is done
+	 * 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
+	 * 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
+

How about simply doing...

	movq	PER_CPU_VAR(current_task), %rax
	movq	$TASK_SIZE_MAX, %rcx
#ifdef CONFIG_BUG_ON_DATA_CORRUPTION
	cmpq	%rcx, TASK_addr_limit(%rax)
	jne	syscall_return_slowpath
#else
	movq	%rcx, TASK_addr_limit(%rax)
#endif

... and let the slow path take care of BUG.  This should be much faster,
even with the BUG, and is simpler to boot.

In fact, we could even to the cmpq/jne unconditionally.  I'm guessing
the occasional branch mispredict will be offset by occasionally touching
a clean cacheline in the case of an unconditional store.

Since this is something that should never happen, performance doesn't
matter.

	-hpa