Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Dmitry Safonov]

On 03/28/2017 03:51 PM, Thomas Gleixner wrote:

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.


Ok, I'll check the ifdeffery, define __NR_{x32_,ia32_}execve,
test it and resend v4 today or tomorrow.
Thanks.



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);





--
 Dmitry

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Dmitry Safonov]

On 03/28/2017 03:51 PM, Thomas Gleixner wrote:

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.


Ok, I'll check the ifdeffery, define __NR_{x32_,ia32_}execve,
test it and resend v4 today or tomorrow.
Thanks.



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);





--
 Dmitry

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Thomas Gleixner]

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);
 

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Thomas Gleixner]

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);
 

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Dmitry Safonov]

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.

--
 Dmitry

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Dmitry Safonov]

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.

--
 Dmitry

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Dmitry Safonov]

On 03/22/2017 01:34 AM, Thomas Gleixner wrote:

On Tue, 21 Mar 2017, h...@zytor.com wrote:


On March 21, 2017 3:21:13 PM PDT, 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.


Indeed, thanks!


Since the i386 syscall namespace is totally separate (and different),
should we simply change the system call number to the appropriate
sys_execve number?


That should work as well and would be more intuitive.


Not sure that I got the idea correctly, something like this?
I haven't find any easy way to get compat syscall nr like
__NR_compat_execve, so I defined it there.
I'll resend v4 with the fixup if that's what was expected.

--->8---
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index b03f186369eb..c58ac0bff2f1 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -507,6 +507,8 @@ void set_personality_64bit(void)
current->personality &= ~READ_IMPLIES_EXEC;
 }

+#define __NR_ia32_execve   11
+
 void set_personality_ia32(bool x32)
 {
/* inherit personality from parent */
@@ -537,6 +539,7 @@ void set_personality_ia32(bool x32)
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;
}
 }

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Dmitry Safonov]

On 03/22/2017 01:34 AM, Thomas Gleixner wrote:

On Tue, 21 Mar 2017, h...@zytor.com wrote:


On March 21, 2017 3:21:13 PM PDT, 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.


Indeed, thanks!


Since the i386 syscall namespace is totally separate (and different),
should we simply change the system call number to the appropriate
sys_execve number?


That should work as well and would be more intuitive.


Not sure that I got the idea correctly, something like this?
I haven't find any easy way to get compat syscall nr like
__NR_compat_execve, so I defined it there.
I'll resend v4 with the fixup if that's what was expected.

--->8---
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index b03f186369eb..c58ac0bff2f1 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -507,6 +507,8 @@ void set_personality_64bit(void)
current->personality &= ~READ_IMPLIES_EXEC;
 }

+#define __NR_ia32_execve   11
+
 void set_personality_ia32(bool x32)
 {
/* inherit personality from parent */
@@ -537,6 +539,7 @@ void set_personality_ia32(bool x32)
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;
}
 }

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Thomas Gleixner]

On Tue, 21 Mar 2017, h...@zytor.com wrote:

> On March 21, 2017 3:21:13 PM PDT, 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.
> >
> >Thanks,
> >
> > tglx
>
> Since the i386 syscall namespace is totally separate (and different),
> should we simply change the system call number to the appropriate
> sys_execve number?

That should work as well and would be more intuitive.

Thanks,

tglx

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Thomas Gleixner]

On Tue, 21 Mar 2017, h...@zytor.com wrote:

> On March 21, 2017 3:21:13 PM PDT, 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.
> >
> >Thanks,
> >
> > tglx
>
> Since the i386 syscall namespace is totally separate (and different),
> should we simply change the system call number to the appropriate
> sys_execve number?

That should work as well and would be more intuitive.

Thanks,

tglx

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[hpa]

On March 21, 2017 3:21:13 PM PDT, 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.
>
>Thanks,
>
>   tglx

Since the i386 syscall namespace is totally separate (and different), should we 
simply change the system call number to the appropriate sys_execve number?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[hpa]

On March 21, 2017 3:21:13 PM PDT, 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.
>
>Thanks,
>
>   tglx

Since the i386 syscall namespace is totally separate (and different), should we 
simply change the system call number to the appropriate sys_execve number?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Thomas Gleixner]

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.

Thanks,

tglx

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Thomas Gleixner]

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.

Thanks,

tglx

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Dmitry Safonov]

2017-03-22 0:16 GMT+03:00 Adam Borowski :
> On Tue, Mar 21, 2017 at 08:47:11PM +0300, Dmitry Safonov wrote:
>> After my changes to mmap(), its code now relies on the bitness of
>> performing syscall. According to that, it chooses the base of allocation:
>> mmap_base for 64-bit mmap() and mmap_compat_base for 32-bit syscall.
>> It was done by:
>>   commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
>> 32-bit mmap()").
>>
>> The code afterwards relies on in_compat_syscall() returning true for
>> 32-bit syscalls. It's usually so while we're in context of application
>> that does 32-bit syscalls. But during exec() it is not valid for x32 ELF.
>> The reason is that the application hasn't yet done any syscall, so x32
>> bit has not being set.
>> That results in -ENOMEM for x32 ELF files as there fired BAD_ADDR()
>> in elf_map(), that is called from do_execve()->load_elf_binary().
>> For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.
>>
>> Set x32 bit before first return to userspace, during setting personality
>> at exec(). This way we can rely on in_compat_syscall() during exec().
>> Do also the reverse: drop x32 syscall bit at SET_PERSONALITY for 64-bits.
>>
>> Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
>> 32-bit mmap()")
>
> Tested:
> with bash:x32, mksh:amd64, posh:i386, zsh:armhf (binfmt:qemu), fork+exec
> works for every parent-child combination.
>
> Contrary to my naive initial reading of your fix, mixing syscalls from a
> process of the wrong ABI also works as it did before.  While using a glibc
> wrapper will call the right version, x32 processes calling amd64 syscalls is
> surprisingly common -- this brings seccomp joy.

JFI: x32 mmap() syscall in 64 ELF should work even better - it has returned
addresses over 4Gb in ia32 mmap()s, so I expect it did the same in x32 top-down
allocation. (I guess you've mentioned the fixes-for patch).
So the thing to check not also that mmap() returned address, but at least
verify-dereference it with `mov' e.g. (or better - to parse /proc/self/maps)

> I've attached a freestanding test case for write() and mmap(); it's
> freestanding asm as most of you don't have an x32 toolchain at hand, sorry
> for unfriendly error messages.
>
> So with these two patches:
> x86/tls: Forcibly set the accessed bit in TLS segments
> x86/mm: set x32 syscall bit in SET_PERSONALITY()
> everything appears to be fine.

Big thanks for the testing work, Adam!

-- 
 Dmitry

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Dmitry Safonov]

2017-03-22 0:16 GMT+03:00 Adam Borowski :
> On Tue, Mar 21, 2017 at 08:47:11PM +0300, Dmitry Safonov wrote:
>> After my changes to mmap(), its code now relies on the bitness of
>> performing syscall. According to that, it chooses the base of allocation:
>> mmap_base for 64-bit mmap() and mmap_compat_base for 32-bit syscall.
>> It was done by:
>>   commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
>> 32-bit mmap()").
>>
>> The code afterwards relies on in_compat_syscall() returning true for
>> 32-bit syscalls. It's usually so while we're in context of application
>> that does 32-bit syscalls. But during exec() it is not valid for x32 ELF.
>> The reason is that the application hasn't yet done any syscall, so x32
>> bit has not being set.
>> That results in -ENOMEM for x32 ELF files as there fired BAD_ADDR()
>> in elf_map(), that is called from do_execve()->load_elf_binary().
>> For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.
>>
>> Set x32 bit before first return to userspace, during setting personality
>> at exec(). This way we can rely on in_compat_syscall() during exec().
>> Do also the reverse: drop x32 syscall bit at SET_PERSONALITY for 64-bits.
>>
>> Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
>> 32-bit mmap()")
>
> Tested:
> with bash:x32, mksh:amd64, posh:i386, zsh:armhf (binfmt:qemu), fork+exec
> works for every parent-child combination.
>
> Contrary to my naive initial reading of your fix, mixing syscalls from a
> process of the wrong ABI also works as it did before.  While using a glibc
> wrapper will call the right version, x32 processes calling amd64 syscalls is
> surprisingly common -- this brings seccomp joy.

JFI: x32 mmap() syscall in 64 ELF should work even better - it has returned
addresses over 4Gb in ia32 mmap()s, so I expect it did the same in x32 top-down
allocation. (I guess you've mentioned the fixes-for patch).
So the thing to check not also that mmap() returned address, but at least
verify-dereference it with `mov' e.g. (or better - to parse /proc/self/maps)

> I've attached a freestanding test case for write() and mmap(); it's
> freestanding asm as most of you don't have an x32 toolchain at hand, sorry
> for unfriendly error messages.
>
> So with these two patches:
> x86/tls: Forcibly set the accessed bit in TLS segments
> x86/mm: set x32 syscall bit in SET_PERSONALITY()
> everything appears to be fine.

Big thanks for the testing work, Adam!

-- 
 Dmitry

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[hpa]

On March 21, 2017 2:16:48 PM PDT, Adam Borowski  wrote:
>On Tue, Mar 21, 2017 at 08:47:11PM +0300, Dmitry Safonov wrote:
>> After my changes to mmap(), its code now relies on the bitness of
>> performing syscall. According to that, it chooses the base of
>allocation:
>> mmap_base for 64-bit mmap() and mmap_compat_base for 32-bit syscall.
>> It was done by:
>>   commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
>> 32-bit mmap()").
>> 
>> The code afterwards relies on in_compat_syscall() returning true for
>> 32-bit syscalls. It's usually so while we're in context of
>application
>> that does 32-bit syscalls. But during exec() it is not valid for x32
>ELF.
>> The reason is that the application hasn't yet done any syscall, so
>x32
>> bit has not being set.
>> That results in -ENOMEM for x32 ELF files as there fired BAD_ADDR()
>> in elf_map(), that is called from do_execve()->load_elf_binary().
>> For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.
>> 
>> Set x32 bit before first return to userspace, during setting
>personality
>> at exec(). This way we can rely on in_compat_syscall() during exec().
>> Do also the reverse: drop x32 syscall bit at SET_PERSONALITY for
>64-bits.
>> 
>> Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
>> 32-bit mmap()")
>
>Tested:
>with bash:x32, mksh:amd64, posh:i386, zsh:armhf (binfmt:qemu),
>fork+exec
>works for every parent-child combination.
>
>Contrary to my naive initial reading of your fix, mixing syscalls from
>a
>process of the wrong ABI also works as it did before.  While using a
>glibc
>wrapper will call the right version, x32 processes calling amd64
>syscalls is
>surprisingly common -- this brings seccomp joy.
>
>I've attached a freestanding test case for write() and mmap(); it's
>freestanding asm as most of you don't have an x32 toolchain at hand,
>sorry
>for unfriendly error messages.
>
>So with these two patches:
>x86/tls: Forcibly set the accessed bit in TLS segments
>x86/mm: set x32 syscall bit in SET_PERSONALITY()
>everything appears to be fine.

What userspace is that?  Is this syscall(3) (ab)users or incorrectly ported to 
x32 software?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[hpa]

On March 21, 2017 2:16:48 PM PDT, Adam Borowski  wrote:
>On Tue, Mar 21, 2017 at 08:47:11PM +0300, Dmitry Safonov wrote:
>> After my changes to mmap(), its code now relies on the bitness of
>> performing syscall. According to that, it chooses the base of
>allocation:
>> mmap_base for 64-bit mmap() and mmap_compat_base for 32-bit syscall.
>> It was done by:
>>   commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
>> 32-bit mmap()").
>> 
>> The code afterwards relies on in_compat_syscall() returning true for
>> 32-bit syscalls. It's usually so while we're in context of
>application
>> that does 32-bit syscalls. But during exec() it is not valid for x32
>ELF.
>> The reason is that the application hasn't yet done any syscall, so
>x32
>> bit has not being set.
>> That results in -ENOMEM for x32 ELF files as there fired BAD_ADDR()
>> in elf_map(), that is called from do_execve()->load_elf_binary().
>> For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.
>> 
>> Set x32 bit before first return to userspace, during setting
>personality
>> at exec(). This way we can rely on in_compat_syscall() during exec().
>> Do also the reverse: drop x32 syscall bit at SET_PERSONALITY for
>64-bits.
>> 
>> Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
>> 32-bit mmap()")
>
>Tested:
>with bash:x32, mksh:amd64, posh:i386, zsh:armhf (binfmt:qemu),
>fork+exec
>works for every parent-child combination.
>
>Contrary to my naive initial reading of your fix, mixing syscalls from
>a
>process of the wrong ABI also works as it did before.  While using a
>glibc
>wrapper will call the right version, x32 processes calling amd64
>syscalls is
>surprisingly common -- this brings seccomp joy.
>
>I've attached a freestanding test case for write() and mmap(); it's
>freestanding asm as most of you don't have an x32 toolchain at hand,
>sorry
>for unfriendly error messages.
>
>So with these two patches:
>x86/tls: Forcibly set the accessed bit in TLS segments
>x86/mm: set x32 syscall bit in SET_PERSONALITY()
>everything appears to be fine.

What userspace is that?  Is this syscall(3) (ab)users or incorrectly ported to 
x32 software?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Adam Borowski]

On Tue, Mar 21, 2017 at 08:47:11PM +0300, Dmitry Safonov wrote:
> After my changes to mmap(), its code now relies on the bitness of
> performing syscall. According to that, it chooses the base of allocation:
> mmap_base for 64-bit mmap() and mmap_compat_base for 32-bit syscall.
> It was done by:
>   commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
> 32-bit mmap()").
> 
> The code afterwards relies on in_compat_syscall() returning true for
> 32-bit syscalls. It's usually so while we're in context of application
> that does 32-bit syscalls. But during exec() it is not valid for x32 ELF.
> The reason is that the application hasn't yet done any syscall, so x32
> bit has not being set.
> That results in -ENOMEM for x32 ELF files as there fired BAD_ADDR()
> in elf_map(), that is called from do_execve()->load_elf_binary().
> For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.
> 
> Set x32 bit before first return to userspace, during setting personality
> at exec(). This way we can rely on in_compat_syscall() during exec().
> Do also the reverse: drop x32 syscall bit at SET_PERSONALITY for 64-bits.
> 
> Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
> 32-bit mmap()")

Tested:
with bash:x32, mksh:amd64, posh:i386, zsh:armhf (binfmt:qemu), fork+exec
works for every parent-child combination.

Contrary to my naive initial reading of your fix, mixing syscalls from a
process of the wrong ABI also works as it did before.  While using a glibc
wrapper will call the right version, x32 processes calling amd64 syscalls is
surprisingly common -- this brings seccomp joy.

I've attached a freestanding test case for write() and mmap(); it's
freestanding asm as most of you don't have an x32 toolchain at hand, sorry
for unfriendly error messages.

So with these two patches:
x86/tls: Forcibly set the accessed bit in TLS segments
x86/mm: set x32 syscall bit in SET_PERSONALITY()
everything appears to be fine.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!
.globl _start
.data
msg:.ascii "Meow!\n"
badmsg: .ascii "syscall failed\n"
.text
_start:
# x32
mov $0x4001, %rax   # syscall: write
mov $1, %rdi
mov $msg, %rsi
mov $6, %rdx
syscall

# amd64
mov $1, %rax# syscall: write
mov $1, %rdi
mov $msg, %rsi
mov $6, %rdx
syscall

# i386
mov $4, %eax# syscall: write
mov $1, %ebx
mov $msg, %ecx
mov $6, %edx
int $0x80


# x32
mov $0x4009, %rax   # syscall: mmap
mov $0, %rdi
mov $0x1, %rsi
mov $3, %rdx# PROT_READ|PROT_WRITE
mov $0x62, %r10 # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %r8
mov $0, %r9
syscall
or  %rax, %rax
js  badness

# amd64
mov $0x9, %rax  # syscall: mmap
mov $0, %rdi
mov $0x1, %rsi
mov $3, %rdx# PROT_READ|PROT_WRITE
mov $0x62, %r10 # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %r8
mov $0, %r9
syscall
or  %rax, %rax
js  badness

jmp goodbye # m'kay, this one doesn't work, no regression
# i386
mov $0x90, %eax # syscall: mmap
mov $0, %ebx
mov $0x1, %ecx
mov $3, %edx# PROT_READ|PROT_WRITE
mov $0x62, %esi # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %edi
mov $0, %ebp
int $0x80
movslq  %eax, %rax
or  %rax, %rax
js  badness

goodbye:
mov $0x403c, %rax   # syscall: _exit
xor %rdi, %rdi
syscall

badness:
# I'm too lazy to printf this as a number...
push%rax
mov $0x4001, %rax   # syscall: write
mov $1, %rdi
mov $badmsg, %rsi
mov $15, %rdx
syscall

mov $0x403c, %rax   # syscall: _exit
pop %rdi
syscall
# Any of amd64/x32/i386 will do.
X86=x86_64-linux-gnu

all: meow-x32 meow-amd64
clean:
rm -f meow-*

meow-x32: meow.s
$(X86)-as --x32 $^ -o $@.o
$(X86)-ld -melf32_x86_64 -s $@.o -o $@

meow-amd64: meow.s
$(X86)-as --64 $^ -o $@.o
$(X86)-ld -melf_x86_64 -s $@.o -o $@

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Adam Borowski]

On Tue, Mar 21, 2017 at 08:47:11PM +0300, Dmitry Safonov wrote:
> After my changes to mmap(), its code now relies on the bitness of
> performing syscall. According to that, it chooses the base of allocation:
> mmap_base for 64-bit mmap() and mmap_compat_base for 32-bit syscall.
> It was done by:
>   commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
> 32-bit mmap()").
> 
> The code afterwards relies on in_compat_syscall() returning true for
> 32-bit syscalls. It's usually so while we're in context of application
> that does 32-bit syscalls. But during exec() it is not valid for x32 ELF.
> The reason is that the application hasn't yet done any syscall, so x32
> bit has not being set.
> That results in -ENOMEM for x32 ELF files as there fired BAD_ADDR()
> in elf_map(), that is called from do_execve()->load_elf_binary().
> For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.
> 
> Set x32 bit before first return to userspace, during setting personality
> at exec(). This way we can rely on in_compat_syscall() during exec().
> Do also the reverse: drop x32 syscall bit at SET_PERSONALITY for 64-bits.
> 
> Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
> 32-bit mmap()")

Tested:
with bash:x32, mksh:amd64, posh:i386, zsh:armhf (binfmt:qemu), fork+exec
works for every parent-child combination.

Contrary to my naive initial reading of your fix, mixing syscalls from a
process of the wrong ABI also works as it did before.  While using a glibc
wrapper will call the right version, x32 processes calling amd64 syscalls is
surprisingly common -- this brings seccomp joy.

I've attached a freestanding test case for write() and mmap(); it's
freestanding asm as most of you don't have an x32 toolchain at hand, sorry
for unfriendly error messages.

So with these two patches:
x86/tls: Forcibly set the accessed bit in TLS segments
x86/mm: set x32 syscall bit in SET_PERSONALITY()
everything appears to be fine.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!
.globl _start
.data
msg:.ascii "Meow!\n"
badmsg: .ascii "syscall failed\n"
.text
_start:
# x32
mov $0x4001, %rax   # syscall: write
mov $1, %rdi
mov $msg, %rsi
mov $6, %rdx
syscall

# amd64
mov $1, %rax# syscall: write
mov $1, %rdi
mov $msg, %rsi
mov $6, %rdx
syscall

# i386
mov $4, %eax# syscall: write
mov $1, %ebx
mov $msg, %ecx
mov $6, %edx
int $0x80


# x32
mov $0x4009, %rax   # syscall: mmap
mov $0, %rdi
mov $0x1, %rsi
mov $3, %rdx# PROT_READ|PROT_WRITE
mov $0x62, %r10 # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %r8
mov $0, %r9
syscall
or  %rax, %rax
js  badness

# amd64
mov $0x9, %rax  # syscall: mmap
mov $0, %rdi
mov $0x1, %rsi
mov $3, %rdx# PROT_READ|PROT_WRITE
mov $0x62, %r10 # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %r8
mov $0, %r9
syscall
or  %rax, %rax
js  badness

jmp goodbye # m'kay, this one doesn't work, no regression
# i386
mov $0x90, %eax # syscall: mmap
mov $0, %ebx
mov $0x1, %ecx
mov $3, %edx# PROT_READ|PROT_WRITE
mov $0x62, %esi # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %edi
mov $0, %ebp
int $0x80
movslq  %eax, %rax
or  %rax, %rax
js  badness

goodbye:
mov $0x403c, %rax   # syscall: _exit
xor %rdi, %rdi
syscall

badness:
# I'm too lazy to printf this as a number...
push%rax
mov $0x4001, %rax   # syscall: write
mov $1, %rdi
mov $badmsg, %rsi
mov $15, %rdx
syscall

mov $0x403c, %rax   # syscall: _exit
pop %rdi
syscall
# Any of amd64/x32/i386 will do.
X86=x86_64-linux-gnu

all: meow-x32 meow-amd64
clean:
rm -f meow-*

meow-x32: meow.s
$(X86)-as --x32 $^ -o $@.o
$(X86)-ld -melf32_x86_64 -s $@.o -o $@

meow-amd64: meow.s
$(X86)-as --64 $^ -o $@.o
$(X86)-ld -melf_x86_64 -s $@.o -o $@