linux-kernel - [tip:x86/urgent] x86/entry/64: Fix stack return address retrieval in thunk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 19 May 2016 02:12:42 -0700
From:	tip-bot for Josh Poimboeuf <tipbot@...or.com>
To:	linux-tip-commits@...r.kernel.org
Cc:	hpa@...or.com, mingo@...nel.org, dvlasenk@...hat.com,
	brgerst@...il.com, torvalds@...ux-foundation.org,
	a.p.zijlstra@...llo.nl, jpoimboe@...hat.com,
	matt@...eblueprint.co.uk, bp@...en8.de, akpm@...ux-foundation.org,
	peterz@...radead.org, tglx@...utronix.de, luto@...nel.org,
	rostedt@...dmis.org, athorlton@....com,
	linux-kernel@...r.kernel.org
Subject: [tip:x86/urgent] x86/entry/64: Fix stack return address retrieval
 in thunk

Commit-ID:  d4bf7078c43e11097e0d6f04d3fb999bf92c4fb0
Gitweb:     http://git.kernel.org/tip/d4bf7078c43e11097e0d6f04d3fb999bf92c4fb0
Author:     Josh Poimboeuf <jpoimboe@...hat.com>
AuthorDate: Tue, 17 May 2016 13:06:06 -0500
Committer:  Ingo Molnar <mingo@...nel.org>
CommitDate: Thu, 19 May 2016 09:12:34 +0200

x86/entry/64: Fix stack return address retrieval in thunk

With CONFIG_FRAME_POINTER enabled, a thunk can pass a bad return address
value to the called function.  '9*8(%rsp)' actually gets the frame
pointer, not the return address.

The only users of the 'put_ret_addr_in_rdi' option are two functions
which trace the enabling and disabling of interrupts, so this bug can
result in bad debug or tracing information with CONFIG_IRQSOFF_TRACER or
CONFIG_PROVE_LOCKING.

Fix this by implementing the suggestion of Linus: explicitly push
the frame pointer all the time and constify the stack offsets that
way. This is both correct and easier to read.

Reported-by: Matt Fleming <matt@...eblueprint.co.uk>
Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
[ Extended the changelog a bit. ]
Acked-by: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Alex Thorlton <athorlton@....com>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Andy Lutomirski <luto@...nel.org>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Brian Gerst <brgerst@...il.com>
Cc: Denys Vlasenko <dvlasenk@...hat.com>
Cc: H. Peter Anvin <hpa@...or.com>
Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Steven Rostedt <rostedt@...dmis.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Fixes: 058fb73274f9 ("x86/asm/entry: Create stack frames in thunk functions")
Link: http://lkml.kernel.org/r/20160517180606.v5o7wcgdni7443ol@treble
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
 arch/x86/entry/thunk_64.S | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S
index 98df1fa..027aec4 100644
--- a/arch/x86/entry/thunk_64.S
+++ b/arch/x86/entry/thunk_64.S
@@ -8,16 +8,15 @@
 #include <linux/linkage.h>
 #include "calling.h"
 #include <asm/asm.h>
-#include <asm/frame.h>
 
 	/* rdi:	arg1 ... normal C conventions. rax is saved/restored. */
 	.macro THUNK name, func, put_ret_addr_in_rdi=0
 	.globl \name
 	.type \name, @function
 \name:
-	FRAME_BEGIN
+	pushq %rbp
+	movq %rsp, %rbp
 
-	/* this one pushes 9 elems, the next one would be %rIP */
 	pushq %rdi
 	pushq %rsi
 	pushq %rdx
@@ -29,8 +28,8 @@
 	pushq %r11
 
 	.if \put_ret_addr_in_rdi
-	/* 9*8(%rsp) is return addr on stack */
-	movq 9*8(%rsp), %rdi
+	/* 8(%rbp) is return addr on stack */
+	movq 8(%rbp), %rdi
 	.endif
 
 	call \func
@@ -65,7 +64,7 @@ restore:
 	popq %rdx
 	popq %rsi
 	popq %rdi
-	FRAME_END
+	popq %rbp
 	ret
 	_ASM_NOKPROBE(restore)
 #endif