2017-03-21 15:59:40

by Dmitry Safonov

[permalink] [raw]
Subject: [PATCH] x86/mm: set x32 syscall bit in SET_PERSONALITY()

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.
For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.

I suggest to set x32 bit before first return to userspace, during
setting personality at exec(). This way we can rely on
in_compat_syscall() during exec().

Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
32-bit mmap()")
Cc: [email protected]
Cc: [email protected]
Cc: Cyrill Gorcunov <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "Kirill A. Shutemov" <[email protected]>
Cc: [email protected]
Cc: H. Peter Anvin <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Reported-by: Adam Borowski <[email protected]>
Signed-off-by: Dmitry Safonov <[email protected]>
---
arch/x86/kernel/process_64.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index d6b784a5520d..88d99d35a699 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -520,7 +520,12 @@ void set_personality_ia32(bool x32)
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 */
+ syscall bit flag to determine compat status.
+ On the bitness of syscall relies x86 mmap() code,
+ so set x32 syscall bit right here to make
+ in_compat_syscall() work during exec().
+ */
+ task_pt_regs(current)->orig_ax |= __X32_SYSCALL_BIT;
current->thread.status &= ~TS_COMPAT;
} else {
set_thread_flag(TIF_IA32);
--
2.12.0


2017-03-21 16:30:08

by Dmitry Safonov

[permalink] [raw]
Subject: Re: [PATCH] x86/mm: set x32 syscall bit in SET_PERSONALITY()

On 03/21/2017 06:55 PM, 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.
> For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.
>
> I suggest to set x32 bit before first return to userspace, during
> setting personality at exec(). This way we can rely on
> in_compat_syscall() during exec().
>
> Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
> 32-bit mmap()")
> Cc: [email protected]
> Cc: [email protected]
> Cc: Cyrill Gorcunov <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: "Kirill A. Shutemov" <[email protected]>
> Cc: [email protected]
> Cc: H. Peter Anvin <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Reported-by: Adam Borowski <[email protected]>
> Signed-off-by: Dmitry Safonov <[email protected]>

Drop this one - I'll send updated v2 shortly slightly improving:
- specifying mmap() allocation path which failed during exec()
- fix comment style (looks like my editor didn't insert asterisks
as they were missing before and check_patch didn't blame me)

> ---
> arch/x86/kernel/process_64.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
> index d6b784a5520d..88d99d35a699 100644
> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -520,7 +520,12 @@ void set_personality_ia32(bool x32)
> 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 */
> + syscall bit flag to determine compat status.
> + On the bitness of syscall relies x86 mmap() code,
> + so set x32 syscall bit right here to make
> + in_compat_syscall() work during exec().
> + */
> + task_pt_regs(current)->orig_ax |= __X32_SYSCALL_BIT;
> current->thread.status &= ~TS_COMPAT;
> } else {
> set_thread_flag(TIF_IA32);
>


--
Dmitry