linux-kernel - Re: [PATCH] x86/asm/64: do not clear high 32 bits of syscall number when CONFIG_X86

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]

Message-ID: <CAE4B646-5A5D-4FB6-89A3-45DD78B64FFD@zytor.com>
Date:   Thu, 14 Sep 2017 22:46:37 -0700
From:   hpa@...or.com
To:     Ingo Molnar <mingo@...nel.org>, Andy Lutomirski <luto@...nel.org>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, X86 ML <x86@...nel.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Eugene Syromyatnikov <evgsyr@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] x86/asm/64: do not clear high 32 bits of syscall number when CONFIG_X86_X32=y

On September 14, 2017 10:31:55 PM PDT, Ingo Molnar <mingo@...nel.org> wrote:
>
>* Andy Lutomirski <luto@...nel.org> wrote:
>
>> >> > diff --git a/arch/x86/entry/entry_64.S
>b/arch/x86/entry/entry_64.S
>> >> > index 4916725..3bab6af 100644
>> >> > --- a/arch/x86/entry/entry_64.S
>> >> > +++ b/arch/x86/entry/entry_64.S
>> >> > @@ -185,12 +185,10 @@ entry_SYSCALL_64_fastpath:
>> >> >          */
>> >> >         TRACE_IRQS_ON
>> >> >         ENABLE_INTERRUPTS(CLBR_NONE)
>> >> > -#if __SYSCALL_MASK == ~0
>> >> > -       cmpq    $__NR_syscall_max, %rax
>> >> > -#else
>> >> > -       andl    $__SYSCALL_MASK, %eax
>> >> > -       cmpl    $__NR_syscall_max, %eax
>> >> > +#if __SYSCALL_MASK != ~0
>> >> > +       andq    $__SYSCALL_MASK, %rax
>> >> >  #endif
>> >> > +       cmpq    $__NR_syscall_max, %rax
>> >>
>> >> I don't know much about x32 userspace, but there's an argument
>that
>> >> the high bits *should* be masked off if the x32 bit is set.
>> >
>> > Why?
>> 
>> Because it always worked that way.
>> 
>> That being said, I'd be okay with applying your patch and seeing
>> whether anything breaks.  Ingo?
>
>So I believe this was introduced with x32 as a 'fresh, modern syscall
>ABI' 
>behavioral aspect, because we wanted to protect the overly complex
>syscall entry 
>code from 'weird' input values. IIRC there was an old bug where we'd
>overflow the 
>syscall table in certain circumstances ...
>
>But our new, redesigned entry code is a lot less complex, a lot more
>readable and 
>a lot more maintainable (not to mention a lot more robust), so if
>invalid RAX 
>values with high bits set get reliably turned into -ENOSYS or such then
>I'd not 
>mind the patch per se either, as a general consistency improvement.
>
>Of course if something in x32 user-land breaks then this turns into an
>ABI and we 
>have to reintroduce this aspect, as a quirk :-/
>
>It should also improve x32 syscall performance a tiny bit, right? So
>might be 
>worth a try on various grounds.
>
>( Another future advantage would be that _maybe_ we could use the high
>RAX 
>component as an extra (64-bit only) special argument of sorts. Not that
>I can 
>  think of any such use right now. )
>
>Thanks,
>
>	Ingo

x32 should be sharing the native 64-but entry code, by design.  We have had the latter mask the upper 32 bits, so we should do that for x32 as well.

There seems to be little if no motivation to ever change both these ABIs.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.