linux-kernel - Re: [PATCHv3] x86/mm: set x32 syscall bit in SET

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:   Tue, 28 Mar 2017 14:51:31 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Dmitry Safonov <dsafonov@...tuozzo.com>
cc:     linux-kernel@...r.kernel.org, 0x7f454c46@...il.com,
        Adam Borowski <kilobyte@...band.pl>, linux-mm@...ck.org,
        Andrei Vagin <avagin@...il.com>,
        Cyrill Gorcunov <gorcunov@...nvz.org>,
        Borislav Petkov <bp@...e.de>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
        Andy Lutomirski <luto@...nel.org>,
        Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

On Tue, 28 Mar 2017, Dmitry Safonov wrote:
> On 03/22/2017 01:21 AM, Thomas Gleixner wrote:
> > On Tue, 21 Mar 2017, Dmitry Safonov wrote:
> > > v3:
> > > - clear x32 syscall flag during x32 -> x86-64 exec() (thanks, HPA).
> > 
> > For correctness sake, this wants to be cleared in the IA32 path as
> > well. It's not causing any harm, but ....
> > 
> > I'll amend the patch.
> 
> So, just a gentle reminder about this problem.
> Should I resend v4 with clearing x32 bit in ia32 path?
> Or should I resend with this fixup:
> https://lkml.org/lkml/2017/3/22/343
> 
> The fixup doesn't look as simple as clearing x32 syscall bit, but I may
> be wrong.

Something like the below should set it correctly for all possible
scenarios.

Thanks,

	tglx

8<------------------

 arch/x86/kernel/process_64.c |   63 ++++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 21 deletions(-)

--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -494,6 +494,8 @@ void set_personality_64bit(void)
 	clear_thread_flag(TIF_IA32);
 	clear_thread_flag(TIF_ADDR32);
 	clear_thread_flag(TIF_X32);
+	/* Pretend that this comes from a 64bit execve */
+	task_pt_regs(current)->orig_ax = __NR_execve;
 
 	/* Ensure the corresponding mm is not marked. */
 	if (current->mm)
@@ -506,32 +508,51 @@ void set_personality_64bit(void)
 	current->personality &= ~READ_IMPLIES_EXEC;
 }
 
-void set_personality_ia32(bool x32)
+static void __set_personality_x32(void)
+{
+#ifdef CONFIG_X86_X32
+	clear_thread_flag(TIF_IA32);
+	set_thread_flag(TIF_X32);
+	if (current->mm)
+		current->mm->context.ia32_compat = TIF_X32;
+	current->personality &= ~READ_IMPLIES_EXEC;
+	/*
+	 * in_compat_syscall() uses the presence of the x32
+	 * syscall bit flag to determine compat status.
+	 * The x86 mmap() code relies on the syscall bitness
+	 * so set x32 syscall bit right here to make
+	 * in_compat_syscall() work during exec().
+	 *
+	 * Pretend to come from a x32 execve.
+	 */
+	task_pt_regs(current)->orig_ax = __NR_x32_execve | __X32_SYSCALL_BIT;
+	current->thread.status &= ~TS_COMPAT;
+#endif
+}
+
+static void __set_personality_ia32(void)
 {
-	/* inherit personality from parent */
+#ifdef CONFIG_COMPAT_32
+	set_thread_flag(TIF_IA32);
+	clear_thread_flag(TIF_X32);
+	if (current->mm)
+		current->mm->context.ia32_compat = TIF_IA32;
+	current->personality |= force_personality32;
+	/* Prepare the first "return" to user space */
+	task_pt_regs(current)->orig_ax = __NR_ia32_execve;
+	current->thread.status |= TS_COMPAT;
+#endif
+}
 
+void set_personality_ia32(bool x32)
+{
 	/* Make sure to be in 32bit mode */
 	set_thread_flag(TIF_ADDR32);
 
-	/* Mark the associated mm as containing 32-bit tasks. */
-	if (x32) {
-		clear_thread_flag(TIF_IA32);
-		set_thread_flag(TIF_X32);
-		if (current->mm)
-			current->mm->context.ia32_compat = TIF_X32;
-		current->personality &= ~READ_IMPLIES_EXEC;
-		/* in_compat_syscall() uses the presence of the x32
-		   syscall bit flag to determine compat status */
-		current->thread.status &= ~TS_COMPAT;
-	} else {
-		set_thread_flag(TIF_IA32);
-		clear_thread_flag(TIF_X32);
-		if (current->mm)
-			current->mm->context.ia32_compat = TIF_IA32;
-		current->personality |= force_personality32;
-		/* Prepare the first "return" to user space */
-		current->thread.status |= TS_COMPAT;
-	}
+	if (x32)
+		__set_personality_x32();
+	else
+		__set_personality_ia32();
 }
 EXPORT_SYMBOL_GPL(set_personality_ia32);