[PATCH 0/3] Some TIF_IA32 fixes

[PATCH 0/3] Some TIF_IA32 fixes

[Andy Lutomirski]

At some point I'd like to remove TIF_IA32 entirely and replace it with
just user_64bit_mode and a per-mm configuration.  In the mean time,
this fixes some incorrect uses.

Ingo, Thomas, Peter, if you like these, please ack the first one and apply
the other two.  The first one will conflict with the pile of entry changes
that I'll send a pull request for in a few days.  (Alternatively, you can
take all three and fix the conflict yourself -- it's trivial.)

Andy Lutomirski (3):
  x86_64, entry: Remove a bogus ret_from_fork optimization
  x86_64, ptrace: Remove checks for TIF_IA32 when changing cs and ss
  x86_64, copy_thread: Fix CLONE_SETTLS bitness

 arch/x86/kernel/entry_64.S   | 13 ++++++++-----
 arch/x86/kernel/process_64.c |  2 +-
 arch/x86/kernel/ptrace.c     | 10 ++--------
 3 files changed, 11 insertions(+), 14 deletions(-)

-- 
2.3.0

[PATCH 1/3] x86_64, entry: Remove a bogus ret_from_fork optimization

[Andy Lutomirski]

ret_from_fork checks TIF_IA32 to determine whether pt_regs and the
related state make sense for ret_from_sys_call.  This is entirely
the wrong check.  TS_COMPAT would make a little more sense, but
there's really no point in keeping this optimization at all.

This fixes a return to the wrong user CS if we came from int 0x80 in
a 64-bit task.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/kernel/entry_64.S | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 9ebaf63ba182..4ee9a2315f80 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -334,11 +334,14 @@ ENTRY(ret_from_fork)
 	testl $3, CS-ARGOFFSET(%rsp)		# from kernel_thread?
 	jz   1f
 
-	testl $_TIF_IA32, TI_flags(%rcx)	# 32-bit compat task needs IRET
-	jnz  int_ret_from_sys_call
-
-	RESTORE_TOP_OF_STACK %rdi, -ARGOFFSET
-	jmp ret_from_sys_call			# go to the SYSRET fastpath
+	/*
+	 * By the time we get here, we have no idea whether our pt_regs,
+	 * ti flags, and ti status came from the 64-bit SYSCALL fast path,
+	 * the slow path, or one of the ia32entry paths.
+	 * Use int_ret_from_sys_call to return, since it can safely handle
+	 * all of the above.
+	 */
+	jmp  int_ret_from_sys_call
 
 1:
 	subq $REST_SKIP, %rsp	# leave space for volatiles
-- 
2.3.0

[PATCH 2/3] x86_64, ptrace: Remove checks for TIF_IA32 when changing cs and ss

[Andy Lutomirski]

The ability for modified cs and/or ss to be useful has nothing to do
with TIF_IA32.  Similarly, if there's an exploit involving changing
cs or ss, it's exploitable with or without a TIF_IA32 check.

So just delete the check.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/kernel/ptrace.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index e510618b2e91..1e125817cf9f 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -364,18 +364,12 @@ static int set_segment_reg(struct task_struct *task,
 	case offsetof(struct user_regs_struct,cs):
 		if (unlikely(value == 0))
 			return -EIO;
-#ifdef CONFIG_IA32_EMULATION
-		if (test_tsk_thread_flag(task, TIF_IA32))
-			task_pt_regs(task)->cs = value;
-#endif
+		task_pt_regs(task)->cs = value;
 		break;
 	case offsetof(struct user_regs_struct,ss):
 		if (unlikely(value == 0))
 			return -EIO;
-#ifdef CONFIG_IA32_EMULATION
-		if (test_tsk_thread_flag(task, TIF_IA32))
-			task_pt_regs(task)->ss = value;
-#endif
+		task_pt_regs(task)->ss = value;
 		break;
 	}
 
-- 
2.3.0

[PATCH 3/3] x86_64, copy_thread: Fix CLONE_SETTLS bitness

[Andy Lutomirski]

CLONE_SETTLS is expected to write a TLS entry in the GDT for 32-bit
callers and to set fsbase for 64-bit callers.

The correct check is is_ia32_task(), which returns true in the
context of a 32-bit syscall.  TIF_IA32 is set if the task itself has
a 32-bit personality, which is not the same thing.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/kernel/process_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 5a2c02913af3..936d43461dca 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -207,7 +207,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,
 	 */
 	if (clone_flags & CLONE_SETTLS) {
 #ifdef CONFIG_IA32_EMULATION
-		if (test_thread_flag(TIF_IA32))
+		if (is_ia32_task())
 			err = do_set_thread_area(p, -1,
 				(struct user_desc __user *)childregs->si, 0);
 		else
-- 
2.3.0

Re: [PATCH 1/3] x86_64, entry: Remove a bogus ret_from_fork optimization

[Ingo Molnar]

* Andy Lutomirski <luto@amacapital.net> wrote:

> ret_from_fork checks TIF_IA32 to determine whether pt_regs and the
> related state make sense for ret_from_sys_call.  This is entirely
> the wrong check.  TS_COMPAT would make a little more sense, but
> there's really no point in keeping this optimization at all.
> 
> This fixes a return to the wrong user CS if we came from int 0x80 in
> a 64-bit task.
> 
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

[tip:x86/asm] x86/ptrace: Remove checks for TIF_IA32 when changing CS and SS

[tip-bot for Andy Lutomirski]

Commit-ID:  08571f1ae327bfb631cb7171bde5ea605df626c6
Gitweb:     http://git.kernel.org/tip/08571f1ae327bfb631cb7171bde5ea605df626c6
Author:     Andy Lutomirski <luto@amacapital.net>
AuthorDate: Tue, 24 Feb 2015 16:01:38 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 25 Feb 2015 08:27:49 +0100

x86/ptrace: Remove checks for TIF_IA32 when changing CS and SS

The ability for modified CS and/or SS to be useful has nothing
to do with TIF_IA32.  Similarly, if there's an exploit involving
changing CS or SS, it's exploitable with or without a TIF_IA32
check.

So just delete the check.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Link: http://lkml.kernel.org/r/71c7ab36456855d11ae07edd4945a7dfe80f9915.1424822291.git.luto@amacapital.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/ptrace.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index e510618..1e12581 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -364,18 +364,12 @@ static int set_segment_reg(struct task_struct *task,
 	case offsetof(struct user_regs_struct,cs):
 		if (unlikely(value == 0))
 			return -EIO;
-#ifdef CONFIG_IA32_EMULATION
-		if (test_tsk_thread_flag(task, TIF_IA32))
-			task_pt_regs(task)->cs = value;
-#endif
+		task_pt_regs(task)->cs = value;
 		break;
 	case offsetof(struct user_regs_struct,ss):
 		if (unlikely(value == 0))
 			return -EIO;
-#ifdef CONFIG_IA32_EMULATION
-		if (test_tsk_thread_flag(task, TIF_IA32))
-			task_pt_regs(task)->ss = value;
-#endif
+		task_pt_regs(task)->ss = value;
 		break;
 	}
 

[tip:x86/asm] x86/ia32-compat: Fix CLONE_SETTLS bitness of copy_thread()

[tip-bot for Andy Lutomirski]

Commit-ID:  72c6fb4f74b6b3797f5b1abd6944d7a1d2adbf04
Gitweb:     http://git.kernel.org/tip/72c6fb4f74b6b3797f5b1abd6944d7a1d2adbf04
Author:     Andy Lutomirski <luto@amacapital.net>
AuthorDate: Tue, 24 Feb 2015 16:01:39 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 25 Feb 2015 08:27:50 +0100

x86/ia32-compat: Fix CLONE_SETTLS bitness of copy_thread()

CLONE_SETTLS is expected to write a TLS entry in the GDT for
32-bit callers and to set FSBASE for 64-bit callers.

The correct check is is_ia32_task(), which returns true in the
context of a 32-bit syscall.  TIF_IA32 is set if the task itself
has a 32-bit personality, which is not the same thing.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Link: http://lkml.kernel.org/r/45e2d0d695393d76406a0c7225b82c76223e0cc5.1424822291.git.luto@amacapital.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/process_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 5a2c029..936d434 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -207,7 +207,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,
 	 */
 	if (clone_flags & CLONE_SETTLS) {
 #ifdef CONFIG_IA32_EMULATION
-		if (test_thread_flag(TIF_IA32))
+		if (is_ia32_task())
 			err = do_set_thread_area(p, -1,
 				(struct user_desc __user *)childregs->si, 0);
 		else