Re: [PATCH 1/2] x86/x32: Use __x64 prefix for X32 compat syscalls

[hpa@zytor.com]

On June 16, 2020 10:17:29 AM PDT, Brian Gerst <brgerst@gmail.com> wrote:
>On Tue, Jun 16, 2020 at 12:49 PM Andy Lutomirski <luto@kernel.org>
>wrote:
>>
>> On Tue, Jun 16, 2020 at 7:23 AM Brian Gerst <brgerst@gmail.com>
>wrote:
>> >
>> > The ABI prefix for syscalls specifies the argument register
>mapping, so
>> > there is no specific reason to continue using the __x32 prefix for
>the
>> > compat syscalls.  This change will allow using native syscalls in
>the X32
>> > specific portion of the syscall table.
>>
>> Okay, I realize that the x86 syscall machinery is held together by
>> duct tape and a lot of luck, but:
>>
>> >
>> > Signed-off-by: Brian Gerst <brgerst@gmail.com>
>> > ---
>> >  arch/x86/entry/syscall_x32.c           |  8 +++-----
>> >  arch/x86/include/asm/syscall_wrapper.h | 10 +++++-----
>> >  2 files changed, 8 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/arch/x86/entry/syscall_x32.c
>b/arch/x86/entry/syscall_x32.c
>> > index 3d8d70d3896c..f993e6254043 100644
>> > --- a/arch/x86/entry/syscall_x32.c
>> > +++ b/arch/x86/entry/syscall_x32.c
>> > @@ -9,15 +9,13 @@
>> >  #include <asm/syscall.h>
>> >
>> >  #define __SYSCALL_64(nr, sym)
>> > +#define __SYSCALL_COMMON(nr, sym) __SYSCALL_X32(nr, sym)
>> >
>> > -#define __SYSCALL_X32(nr, sym) extern long __x32_##sym(const
>struct pt_regs *);
>> > -#define __SYSCALL_COMMON(nr, sym) extern long __x64_##sym(const
>struct pt_regs *);
>> > +#define __SYSCALL_X32(nr, sym) extern long __x64_##sym(const
>struct pt_regs *);
>> >  #include <asm/syscalls_64.h>
>> >  #undef __SYSCALL_X32
>> > -#undef __SYSCALL_COMMON
>> >
>> > -#define __SYSCALL_X32(nr, sym) [nr] = __x32_##sym,
>> > -#define __SYSCALL_COMMON(nr, sym) [nr] = __x64_##sym,
>> > +#define __SYSCALL_X32(nr, sym) [nr] = __x64_##sym,
>> >
>> >  asmlinkage const sys_call_ptr_t
>x32_sys_call_table[__NR_x32_syscall_max+1] = {
>> >         /*
>> > diff --git a/arch/x86/include/asm/syscall_wrapper.h
>b/arch/x86/include/asm/syscall_wrapper.h
>> > index a84333adeef2..267fae9904ff 100644
>> > --- a/arch/x86/include/asm/syscall_wrapper.h
>> > +++ b/arch/x86/include/asm/syscall_wrapper.h
>> > @@ -17,7 +17,7 @@ extern long __ia32_sys_ni_syscall(const struct
>pt_regs *regs);
>> >   * __x64_sys_*()         - 64-bit native syscall
>> >   * __ia32_sys_*()        - 32-bit native syscall or common compat
>syscall
>> >   * __ia32_compat_sys_*() - 32-bit compat syscall
>>
>> On a 64-bit kernel, an "ia32" compat syscall is __ia32_compat_sys_*,
>but...
>>
>> > - * __x32_compat_sys_*()  - 64-bit X32 compat syscall
>> > + * __x64_compat_sys_*()  - 64-bit X32 compat syscall
>>
>> Now an x32 compat syscall is __x64_compat?  This seems nonsensical.
>
>Again, think of it as how the registers are mapped, not which syscall
>table it belongs to.  X32 and X64 are identical in that regard.
>
>> I'm also a bit confused as to how this is even necessary for your
>> other patch.
>
>This came out of discussion on Cristoph's patch to combine compat
>execve*() into the native version:
>https://lore.kernel.org/lkml/20200615141239.GA12951@lst.de/
>
>The bottom line is that marking a syscall as X32-only in the syscall
>table forces an __x32 prefix even if it's not a "compat" syscall.
>This causes a link failure.  This is just another quirk caused by how
>X32 was designed.  The solution is to make the prefix consistent for
>the whole table.  The other alternative is to use __x32 for all the
>common syscalls.
>
>The second patch isn't really necessary, but it makes more sense to
>not have a compat syscall with no corresponding native version.
>
>--
>Brian Gerst

Please don't use "x64" to mean anything other than x86-64, as some, ahem, other OSes use those as synonyms.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.