linux-kernel - Re: [PATCH v2 5/9] x86: elf: Use e_machine to select start

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:   Thu, 1 Oct 2020 14:49:28 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Gabriel Krisman Bertazi <krisman@...labora.com>
Cc:     Andrew Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Christoph Hellwig <hch@....de>,
        "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>,
        Robert Richter <rric@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, X86 ML <x86@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>, kernel@...labora.com
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
<krisman@...labora.com> 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 <krisman@...labora.com>
> ---
>  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