2020-10-01 21:01:20

by Gabriel Krisman Bertazi

Subject: [PATCH v2 5/9] x86: elf: Use e_machine to select start_thread for x32

Since TIF_X32 is going away, avoid using it to find the ELF type on
compat_start_thread

According to SysV AMD64 ABI Draft, an AMD64 ELF object using ILP32 must
have ELFCLASS32 with (E_MACHINE == EM_X86_64), so use that ELF field to
differentiate a x32 object from a IA32 object when executing
start_thread in compat mode.

Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
arch/x86/include/asm/elf.h | 11 +++++++++--
arch/x86/kernel/process_64.c | 11 +++++++----
2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 9220efc65d78..33c1c9be2e07 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -186,8 +186,15 @@ static inline void elf_common_init(struct thread_struct *t,
#define COMPAT_ELF_PLAT_INIT(regs, load_addr) \
elf_common_init(&current->thread, regs, __USER_DS)

-void compat_start_thread(struct pt_regs *regs, u32 new_ip, u32 new_sp);
-#define compat_start_thread compat_start_thread
+void compat_start_thread_ia32(struct pt_regs *regs, u32 new_ip, u32 new_sp);
+void compat_start_thread_x32(struct pt_regs *regs, u32 new_ip, u32 new_sp);
+#define compat_start_thread(regs, new_ip, new_sp) \
+do { \
+ if (elf_ex->e_machine == EM_X86_64) \
+ compat_start_thread_x32(regs, new_ip, new_sp); \
+ else \
+ compat_start_thread_ia32(regs, new_ip, new_sp); \
+} while (0)

void set_personality_ia32(bool);
#define COMPAT_SET_PERSONALITY(ex) \
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 9afefe325acb..56e882c339e6 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -511,12 +511,15 @@ start_thread(struct pt_regs *regs, unsigned long new_ip, unsigned long new_sp)
EXPORT_SYMBOL_GPL(start_thread);

#ifdef CONFIG_COMPAT
-void compat_start_thread(struct pt_regs *regs, u32 new_ip, u32 new_sp)
+void compat_start_thread_ia32(struct pt_regs *regs, u32 new_ip, u32 new_sp)
{
start_thread_common(regs, new_ip, new_sp,
- test_thread_flag(TIF_X32)
- ? __USER_CS : __USER32_CS,
- __USER_DS, __USER_DS);
+ __USER32_CS, __USER_DS, __USER_DS);
+}
+void compat_start_thread_x32(struct pt_regs *regs, u32 new_ip, u32 new_sp)
+{
+ start_thread_common(regs, new_ip, new_sp,
+ __USER_CS, __USER_DS, __USER_DS);
}
#endif

--
2.28.0

2020-10-01 21:51:29

by Andy Lutomirski

Subject: Re: [PATCH v2 5/9] x86: elf: Use e_machine to select start_thread for x32

On Thu, Oct 1, 2020 at 1:59 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> Since TIF_X32 is going away, avoid using it to find the ELF type on
> compat_start_thread
>
> According to SysV AMD64 ABI Draft, an AMD64 ELF object using ILP32 must
> have ELFCLASS32 with (E_MACHINE == EM_X86_64), so use that ELF field to
> differentiate a x32 object from a IA32 object when executing
> start_thread in compat mode.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
> ---
> arch/x86/include/asm/elf.h | 11 +++++++++--
> arch/x86/kernel/process_64.c | 11 +++++++----
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> index 9220efc65d78..33c1c9be2e07 100644
> --- a/arch/x86/include/asm/elf.h
> +++ b/arch/x86/include/asm/elf.h
> @@ -186,8 +186,15 @@ static inline void elf_common_init(struct thread_struct *t,
> #define COMPAT_ELF_PLAT_INIT(regs, load_addr) \
> elf_common_init(&current->thread, regs, __USER_DS)
>
> -void compat_start_thread(struct pt_regs *regs, u32 new_ip, u32 new_sp);
> -#define compat_start_thread compat_start_thread
> +void compat_start_thread_ia32(struct pt_regs *regs, u32 new_ip, u32 new_sp);
> +void compat_start_thread_x32(struct pt_regs *regs, u32 new_ip, u32 new_sp);
> +#define compat_start_thread(regs, new_ip, new_sp) \
> +do { \
> + if (elf_ex->e_machine == EM_X86_64) \
> + compat_start_thread_x32(regs, new_ip, new_sp); \
> + else \
> + compat_start_thread_ia32(regs, new_ip, new_sp); \
> +} while (0)

This is evil -- it looks like a real function, but it's not. Can you
instead add a const struct elf32_hdr *elf_ex parameter to all the
compat_start_thread implementations? There appear to be only four of
them in the whole kernel. For patches like this, it should be fine to
do it all as one patch as long as you Cc all the arch maintainers.

--Andy